feat(backend,prepro): use etag to reduce database load #2768
Conversation
This file contains 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
corneliusroemer
force-pushed
the
etag-prepro
branch
from
cda70e5
to
031a4bb
Compare
corneliusroemer
force-pushed
the
etag-prepro
branch
from
147be35
to
7016c49
Compare
corneliusroemer
force-pushed
the
table-update-tracker
branch
from
a870345
to
ef6ca41
Compare
corneliusroemer
force-pushed
the
etag-prepro
branch
from
7016c49
to
b26c920
Compare
anna-parker
requested changes
Sep 13, 2024
There was a problem hiding this comment.
Looks good! Only two places I am a bit unsure about.
Also does this mean that the database query in fetchUnprocessedEntriesAndUpdateToInProcessing was just really inefficient? Because the main change that this PR introduces is that if there has not been a change in the database we do not perform this complicated select entries in sequence_entries where ... not in preprocessing and do not stream an empty response.
backend/src/main/kotlin/org/loculus/backend/controller/SubmissionController.kt
Show resolved
Hide resolved
anna-parker
reviewed
Sep 13, 2024
anna-parker
approved these changes
Sep 13, 2024
corneliusroemer
force-pushed
the
etag-prepro
branch
from
0ad25fb
to
e2e4d7b
Compare
corneliusroemer
commented
Sep 19, 2024
backend/src/main/kotlin/org/loculus/backend/model/ReleasedDataModel.kt
Outdated
Show resolved
Hide resolved
corneliusroemer
requested review from
fengelniederhammer,
theosanderson and
chaoran-chen
anna-parker
reviewed
Sep 19, 2024
| @@ -135,6 +134,7 @@ open class SubmissionController( | |||
| ), | |||
| ], | |||
| ) | |||
| @ApiResponse(responseCode = "304", description = "Not Modified") | |||
There was a problem hiding this comment.
lets maybe add a test for this - we can basically just copy the test we had for get-released-data?
co-authored with @anna-parker and Claude Sonnet try now it might work? Remove views fixup Change to a conditional update for useNewerProcessingPipelineIfPossible, add last_updated_db as a view. (#2741) Co-authored-by: Anna (Anya) Parker <50943381+anna-parker@users.noreply.github.com> Still return current newest version Return version from correct place Don't issue delete statement unless there's something to delete fix ktlint Adjust useNewerProcessingPipelineIfPossible to only update table when there are changes Table update tracker anya (#2764) * Add option to get-released-data to check last update time. * make name better * Make or short-circuiting * Only work with table_update_tracker - return lastTime in LAST_MODIFIED header * Remove unused functions * add back missing imports - somehow format removed this??? * Update bash scripts * Attempt to fix bash * 2nd little fix * echo value of last_modified not name * Do not update SILO if loculus responds with 304. * Always echo $lastSnapshot unless new data existed in loculus AND this was successfully sent to LAPIS - then echo $last_modified * save last snapshot to file * Update with loculus data anyways if longer than 1hour since last update * fix * Improve silo import scripts etc * add back BACKEND_BASE_URL * Log headers * Regularize * Better logs * Try to fix error * return 0 and never null in header * Fix read non-existent file bug * Better wrapper logging * fix delete after update snapshot * Make grep case insensitive * add back removed exit 0 * Improvement to logging * Add test * Add full process to test * Update documentation --------- Co-authored-by: Cornelius Roemer <cornelius.roemer@gmail.com> CR edits to table-update-tracker (#2767) * Add option to get-released-data to check last update time. * make name better * Make or short-circuiting * Only work with table_update_tracker - return lastTime in LAST_MODIFIED header * Remove unused functions * add back missing imports - somehow format removed this??? * Update bash scripts * Attempt to fix bash * 2nd little fix * echo value of last_modified not name * Do not update SILO if loculus responds with 304. * Always echo $lastSnapshot unless new data existed in loculus AND this was successfully sent to LAPIS - then echo $last_modified * save last snapshot to file * Update with loculus data anyways if longer than 1hour since last update * fix * Improve silo import scripts etc * add back BACKEND_BASE_URL * Log headers * Regularize * Better logs * Try to fix error * return 0 and never null in header * Fix read non-existent file bug * Better wrapper logging * fix delete after update snapshot * Make grep case insensitive * add back removed exit 0 * Improvement to logging * Add test * Add full process to test * Update documentation * Use etag instead of modified-since for spec compliance * ktlin fix * Move logic into model * Fix etag formatting * Fix etag yet again --------- Co-authored-by: Anna (Anya) Parker <50943381+anna-parker@users.noreply.github.com> refactor(backend): streamline control flow refactor(backend): extract function Update backend/src/main/kotlin/org/loculus/backend/controller/SubmissionController.kt Co-authored-by: Fabian Engelniederhammer <92720311+fengelniederhammer@users.noreply.github.com> Update backend/src/main/kotlin/org/loculus/backend/controller/SubmissionController.kt Co-authored-by: Fabian Engelniederhammer <92720311+fengelniederhammer@users.noreply.github.com> Update backend/src/test/kotlin/org/loculus/backend/controller/submission/GetReleasedDataEndpointTest.kt Co-authored-by: Fabian Engelniederhammer <92720311+fengelniederhammer@users.noreply.github.com> Enable etag for /extract-unprocessed-data Use etag in prepro to skip preprocessing match case refactor Try fix fix logic fix! i'm stupid Force refresh every hour Add response code annotations Update preprocessing/nextclade/src/loculus_preprocessing/prepro.py Co-authored-by: Anna (Anya) Parker <50943381+anna-parker@users.noreply.github.com> Add etag to dummy preprocessing # Conflicts: # backend/src/main/kotlin/org/loculus/backend/controller/SubmissionController.kt # backend/src/main/kotlin/org/loculus/backend/controller/SubmissionControllerDescriptions.kt # backend/src/main/kotlin/org/loculus/backend/model/ReleasedDataModel.kt # backend/src/main/kotlin/org/loculus/backend/service/submission/SubmissionDatabaseService.kt # backend/src/main/kotlin/org/loculus/backend/service/submission/UpdateTrackerTable.kt # backend/src/main/resources/db/migration/V1.2__add_table_update_tracker.sql # backend/src/test/kotlin/org/loculus/backend/controller/submission/GetReleasedDataEndpointTest.kt # kubernetes/loculus/silo_import_job.sh
corneliusroemer
force-pushed
the
etag-prepro
branch
from
b42c86c
to
d968edc
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
relates to #2092
preview URL: https://etag-prepro.loculus.org
Summary
Send etag to /extract-unprocessed-sequences to reduce db/backend load
Screenshot
Big reduction in db and backend load, approximately to 1/4 to 1/3 of the previous base load
PR Checklist