Log EventDataIds through the SND event and attribute data ETL steps - #985
Merged
Merged
Conversation
The _SND Event Data step clears attribute values and only the _SND Attribute Data step restores them, and each step picks its rows independently, so a mismatch silently leaves event data with no attributes. Logging the ids on both sides makes the gap visible in the job log.
The exp.Object lookup added for this branch's id logging ran unguarded inside mergeRows, so a failure reading it would abort a merge that previously did no reads there at all. It now runs inside logAttributeDataToBeCleared, which logs a warning and lets the merge continue. The lookup is also skipped once the batch exceeds MAX_LOGGED_IDS, the same threshold above which logIds suppresses the id list, capping the chunked queries at ten round trips instead of the hundreds a full merge batch would spend to produce a bare count. MAX_LOGGED_IDS is public now so both sites share one constant.
These two lines carried the property name but nothing identifying the row, so a single attribute dropped from an otherwise healthy EventDataId could not be joined back to the id sets the rest of this branch logs. Per-attribute loss was invisible to every one of them, since they all reconcile at EventDataId granularity and an id with any resolving value counts as written.
The event data step logged ids only from mergeRows, and only the subset that already carried attribute values, so there was nothing to diff against the id sets the attribute data step logs. importRows and deleteRows now log theirs as well: a newly inserted row the _SND Attribute Data step misses ends up just as stripped as a cleared one, and a deleted row otherwise reads as an attribute data miss. Everything this branch adds now logs at debug, which the ETL job logger runs at by default. The pre-existing Begin/End updating exp.ObjectProperty lines are back at info, and the two diagnostics that cost real work - the exp.Object lookup for values about to be cleared, and the source ordering check - are skipped entirely when debug is off. MAX_LOGGED_IDS drops to 2000, below the 5000 row ETL batch size, so the full batches of an initial load suppress the id lists while the smaller batches of an incremental run keep them.
The two source views derive their rowversion differently - v_snd_eventData from CODED_PROCS alone, v_snd_attributeData from MAX over CODED_PROCS and CODED_PROC_ATTRIBS - and each step clamps its own window end to MIN_ACTIVE_ROWVERSION at the moment it runs, so the two incremental windows can cover different rows even though the values sit on one sequence. Logging the span a batch actually covered puts that next to the window the ETL already logs. The attribute step also pairs each EventDataId it left unwritten with its rowversion, on a line of its own so the bare id list stays diffable against the event step's. An id whose rowversion falls inside the window was not excluded by it: it is missing from v_snd_attributeData altogether, which the inner joins on CODED_PROC_ATTRIBS and PKG_ATTRIBS and the blank-value filter can each cause, and which does not heal on the next run. The delete path gets no span; its source view filters on audit_date_tm rather than the rowversion column.
labkey-martyp
requested review from
RameshRapa,
jallentxbiomed and
labkey-bpatel
August 24, 2026 15:04
…fb_snd_etl_id_logging
jallentxbiomed
approved these changes
Aug 26, 2026
ankurjuneja
approved these changes
Aug 27, 2026
The rowversion span logged by the previous commit never appeared: TransformDataIteratorBuilder drops every source column SQL Server types as a rowversion, so the incremental filter column itself never reaches the steps that log it and every batch reported no rowversions. Both source views now expose their rowversions a second time as BIGINT, which passes through untouched. The filter columns are unchanged, so the incremental windows are unaffected. The attribute data view exposes the coded proc row's and the attribute row's rowversions separately rather than only the MAX it filters on, and each unwritten EventDataId is now marked with the side its rowversion came from. An (a) id is newer on the attribute row than on the coded proc row the event step filtered on, so this step's window can exclude an id that step just cleared and the next run restores it; a (p) id carries the value the event step saw, so the window is not what kept it out and no later run brings it back. The views deploy separately from the module, so until they are updated both helpers report that the source view is missing the BIGINT copies rather than logging a column of nulls.
jallentxbiomed
approved these changes
Sep 1, 2026
…fb_snd_etl_id_logging
labkey-martyp
added a commit
that referenced
this pull request
Sep 16, 2026
## Rationale Stop the SND ETL losing attribute values when the event data and attribute data steps' incremental windows drift apart, and make a blanked source attribute clear its stored value instead of leaving it stale. The event data step deleted and recreated each merged row's exp object, which discarded its attribute values, and only the attribute data step restores them; because that step computes its own incremental window, a row whose change landed in the seconds between the two steps computing their windows was cleared by the first and never seen by the second, and its values were gone permanently. The delete dates from the days before the attribute data step existed and has been redundant since that step began replacing values property by property. The attribute data source view also filtered blank values out, so a cleared attribute never reached the ETL and its old value survived. That was masked by the same delete, which is why it surfaces alongside this fix. The view change is a source-database ALTER VIEW and deploys separately from the module. The module change is safe to deploy first, since no null-valued rows arrive until the view changes. ## Related Pull Requests - #985 added the per-step EventDataId logging that pinpointed the lost rows. ## Changes - The event data merge no longer deletes and recreates each row's exp object, so attribute values survive a merge that the attribute data step's window does not also cover. - The attribute data source view passes blank values through as null-valued rows instead of filtering them out, so a cleared attribute reaches the ETL. - The attribute data step removes the stored value for a null-valued source row; a lookup value that fails to resolve still leaves the old value in place. - The diagnostic that reported which values a merge was about to clear is removed, since nothing is cleared now.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale
Make it possible to tell from the ETL job log which event data rows ended up holding no attribute values. The _SND Event Data step clears attribute values as a side effect of its merge and only the _SND Attribute Data step restores them; because the two steps decide independently which rows to process, a mismatch leaves event data silently stripped. Newly inserted rows carry the same exposure, arriving with no attribute values and depending on that same second step to supply them. Until now nothing in the log identified the rows either step handled, so the problem could only be found by querying the database after the fact.
Related Pull Requests
None.
Changes