[LNT] Fixed fieldchanges grouping to a regression

[kpdev42]

Summary

Note active_indicators is not a list of regression indicators. It is a list of fieldchanges actually.
After that fix all new fieldchanges will be grouped mostly in the same regression as expected.
It prevents creating of thousand new regressions and ageing them out immediately to keep regressions count under 50.

[thopre]

lnt/server/db/fieldchange.py

269

Why is this change correct / needed?

[slydiman]

lnt/server/db/fieldchange.py

269

It seems 2 months ago it was a bug fix for missing session.flush() in line 175.
Now it is just a performance optimization.
Note the method new_regression() allows the fieldchange id (requires additional query to DB) or the fieldchange object itself (faster).

[thopre]

LGTM with the description update

lnt/server/db/fieldchange.py

269

Ah I see. Would you mind mentioning it in the patch description?

[kpdev42]

@slydiman Could you please commit this patch?

[thopre]

In D109216#2991145, @kpdev42 wrote:

@slydiman Could you please commit this patch?

I've landed it for you

lnt/server/db/fieldchange.py

changes_of_interest = set(session.query(ts.FieldChange.start_order_id,
                                             ts.FieldChange.field_id)
                               .filter(ts.FieldChange.start_order == start_order)
                               .filter(ts.FieldChange.end_order == end_order)
                               .filter(ts.FieldChange.test_id.in_(runinfo.test_ids))
                               .filter(ts.FieldChange.machine == run.machine)
                               .filter(ts.FieldChange.field_id.in_(field_ids))
                               .all())
 
-    active_indicators = session.query(ts.FieldChange) \
+    active_changes = session.query(ts.FieldChange) \
         .join(ts.RegressionIndicator) \
         .join(ts.Regression) \
         .filter(or_(ts.Regression.state == RegressionState.DETECTED,
                     ts.Regression.state == RegressionState.DETECTED_FIXED)) \
         .options(joinedload(ts.FieldChange.start_order),
                  joinedload(ts.FieldChange.end_order),
                  joinedload(ts.FieldChange.test),
                  joinedload(ts.FieldChange.machine)) \
for field in list(ts.Sample.get_metric_fields()):
                                    end_order=run.order,
                                    machine=run.machine,
                                    test=test,
                                    field_id=field.id)
                 session.add(f)
                 session.flush()
                 try:
                     found, new_reg = identify_related_changes(session, ts,
-                                                              f, active_indicators)
+                                                              f, active_changes)
                 except ObjectDeletedError:
                     # This can happen from time to time.
                     # So, lets retry once.
                     found, new_reg = identify_related_changes(session, ts,
-                                                              f, active_indicators)
+                                                              f, active_changes)
 
                 if found:
                     logger.info("Found field change: {}".format(
                                 run.machine))
 
             # Always update FCs with new values.
             if f:
                 f.old_value = result.previous
def percent_similar(a, b):
     :param a: first string
     :param b: second string
     """
     s = difflib.SequenceMatcher(lambda x: x.isdigit(), a, b)
     return s.ratio()
 
 
 @timed
-def identify_related_changes(session, ts, fc, active_indicators):
+def identify_related_changes(session, ts, fc, active_changes):
     # type: (Session, TestSuiteDB, TestSuiteDB.FieldChange, List) -> Tuple[bool, List]
     """Can we find a home for this change in some existing regression? If a
     match is found add a regression indicator adding this change to that
     regression, otherwise create a new regression for this change.
 
     Regression matching looks for regressions that happen in overlapping order
     ranges. Then looks for changes that are similar.
 
     """
 
-    for change in active_indicators:
+    for change in active_changes:
         if is_overlaping(change, fc):
             confidence = 0.0
 
             confidence += percent_similar(change.machine.name,
                                           fc.machine.name)
             confidence += percent_similar(change.test.name,
                                           fc.test.name)
 
             if change.field_id == fc.field_id:
                 confidence += 1.0
 
             if confidence >= 2.0:
                 # Matching
                 MSG = "Found a match: {} with score {}."
                 regression = session.query(ts.Regression) \
                     .join(ts.RegressionIndicator) \
                     .filter(ts.RegressionIndicator.field_change_id == change.id) \
                     .one()
                 logger.info(MSG.format(str(regression),
                                        confidence))
                 ri = ts.RegressionIndicator(regression, fc)
                 session.add(ri)
                 session.flush()
-                active_indicators.append(ri)
+                active_changes.append(fc)
                 # Update the default title if needed.
                 rebuild_title(session, ts, regression)
                 logger.info("Updated title of Regression({}) to \"{}\"".format(regression.id, regression.title))
                 return True, regression
 
-    new_reg, new_indicators = new_regression(session, ts, [fc.id])
+    new_reg, new_indicators = new_regression(session, ts, [fc])
     logger.info("Could not find a partner, creating new Regression for change: {}".format(new_reg.title))
-    active_indicators.extend(new_indicators)
+    active_changes.append(fc)
     return False, new_reg