Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If the ingest worker tries to update a non-existent ingest, retry #739

Merged
merged 13 commits into from
Sep 18, 2020

Conversation

alexwlchan
Copy link
Contributor

Closes wellcomecollection/platform#4781

This introduces a new status code in the ingest tracker API. If you do a PATCH /ingest/{id} and that ingest doesn't exist yet, you get an HTTP 404 Not Found instead of HTTP 409 Conflict. This can occur if there's a DynamoDB consistency issue that means the DynamoTracker can't find the ingest.

The tracker client turns this status code into a new error type, and the ingest worker treats this error as non-deterministic.

Plus a couple of other changes to make this code easier to debug if we have other issues in future.

@alexwlchan alexwlchan requested a review from a team September 18, 2020 11:07
@alexwlchan alexwlchan force-pushed the allow-multi-ingest-tracker-updates branch from 89a5ca5 to 475aa67 Compare September 18, 2020 11:07
Comment on lines -40 to -43
case class IngestStoreUnexpectedError(e: Throwable) extends IngestStoreError {

case class IngestStoreUnexpectedError(storageError: StorageError)
extends IngestStoreError {
override def toString: String = {
s"IngestStoreUnexpectedError: ${e.toString}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discarding the outer wrapper meant the Throwable we got was sometimes null, which is unhelpful.

Copy link
Contributor

@alicefuzier alicefuzier left a comment

Choose a reason for hiding this comment

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

🕵️

@alexwlchan alexwlchan merged commit 626b1c1 into master Sep 18, 2020
@alexwlchan alexwlchan deleted the allow-multi-ingest-tracker-updates branch September 21, 2020 06:04
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.

The ingest tracker drops updates if they come in too close together
3 participants