Skip to content

Conversation

@bbimber
Copy link
Collaborator

@bbimber bbimber commented Mar 15, 2024

@labkey-martyp, this is tangential to #730. Now that the kinship process is actually working, I forgot how long the insert takes. Matt keyed me in to using PreparedStatement instead of looping over Table.insert(). This should be a minor change that improves insert perf.

@bbimber bbimber changed the base branch from develop to release23.11-SNAPSHOT March 15, 2024 20:13
Copy link
Contributor

@labkey-martyp labkey-martyp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. Thanks.

@bbimber
Copy link
Collaborator Author

bbimber commented Mar 18, 2024

hi @labkey-martyp, I just added one additional small change. this commit allows the job to monitor for being cancelled, and respect that. you already approved, but given this is a change I thought I'd ping you on it.

Also, I added a sort on the TSV produced by the kinship R script. This should have no functional change with import now, but it occurred to me that given how rarely these data change, it is probably unnecessary to constantly re-import everything. If PreparedStatement isnt enough of a speed boost, the other option would be to read two streams: one from the TSV and one from the kinship table with existing records. If those are sorted on Id/Id2, it would be pretty easy to walk those and only do the import/update/delete for records that have actually changed.

@bbimber
Copy link
Collaborator Author

bbimber commented Mar 20, 2024

hey @labkey-martyp: can you confirm you're still good with this? you may not have seen, but I added a couple minor changes since the first approval. github will let me merge but doesnt seem in the spirit of review. note: these test failures to not seem related and the tests to exercise kinship seem to have passed.

@labkey-martyp
Copy link
Contributor

@bbimber Your changes look good. Thanks. Yeah looks like those are just flaky test failures not related to this PR.

@bbimber bbimber merged commit 18b219b into release23.11-SNAPSHOT Mar 20, 2024
@bbimber bbimber deleted the 23.11_fb_kinship_perf branch March 20, 2024 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants