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

Use job queue to ensure eventual synchronization of deleted annotations from Postgres to Elasticsearch #7840

Closed
robertknight opened this issue Feb 6, 2023 · 1 comment · Fixed by #8564
Assignees
Labels

Comments

@robertknight
Copy link
Member

We have found several instances (hypothesis/client#5219, #7796) of annotation deletions not being synchronized from Postgres to Elasticsearch. So far the issue seems to happen infrequently, but when it does, it causes confusion due to mismatches between the search result count and actual number of annotations, and in API clients it can cause them to fail to paginate properly through results (see hypothesis/client#5219, although we need a separate solution for this).

We have a system in place to ensure that annotations are guaranteed to be eventually from Postgres => Elasticsearch after being created/updated, even in the event of an outage of Elasticsearch or RabbitMQ. This ticket is about using that system to improve reliability of deletion sync as well.

Original context: https://hypothes-is.slack.com/archives/C4K6M7P5E/p1675687184055039

robertknight added a commit to hypothesis/client that referenced this issue Feb 7, 2023
There is currently an issue in h where the search API call may sometimes return
pages that are incomplete, even if there are more pages of results to fetch
afterwards [1]. Until this is resolved in the backend, change the client to
continue paging through results as long as the current page has at least one
entry, which we need to get the cursor for the next page, and we expect more
results based on the `total` value.

Fixes #5219

[1] hypothesis/h#7840
robertknight added a commit to hypothesis/client that referenced this issue Feb 7, 2023
There is currently an issue in h where the search API call may sometimes return
pages that are incomplete, even if there are more pages of results to fetch
afterwards [1]. A non-full page caused the client to stop fetching more pages as
it incorrectly assumed it had reached the end. Ultimately this needs to be
resolved in the API [2] by adding an explicit next-page link. Until that is done,
change the client to continue paging through results as long as the current page
has at least one entry (needed to construct the cursor) and we expect more
results based on the `total` value in the response.

Fixes #5219

[1] hypothesis/h#7840
[2] hypothesis/h#7841
robertknight added a commit to hypothesis/client that referenced this issue Feb 7, 2023
There is currently an issue in h where the search API call may sometimes return
pages that are incomplete, even if there are more pages of results to fetch
afterwards [1]. A non-full page caused the client to stop fetching more pages as
it incorrectly assumed it had reached the end. Ultimately this needs to be
resolved in the API [2] by adding an explicit next-page link. Until that is done,
change the client to continue paging through results as long as the current page
has at least one entry (needed to construct the cursor) and we expect more
results based on the `total` value in the response.

Fixes #5219

[1] hypothesis/h#7840
[2] hypothesis/h#7841
robertknight added a commit to hypothesis/client that referenced this issue Feb 8, 2023
There is currently an issue in h where the search API call may sometimes return
pages that are incomplete, even if there are more pages of results to fetch
afterwards [1]. A non-full page caused the client to stop fetching more pages as
it incorrectly assumed it had reached the end. Ultimately this needs to be
resolved in the API [2] by adding an explicit next-page link. Until that is done,
change the client to continue paging through results as long as the current page
has at least one entry (needed to construct the cursor) and we expect more
results based on the `total` value in the response.

Fixes #5219

[1] hypothesis/h#7840
[2] hypothesis/h#7841
@leedenison leedenison added the bug label May 19, 2023
@nairiboo nairiboo added the S4 label Jul 7, 2023
@seanh
Copy link
Contributor

seanh commented Feb 11, 2024

I'll also add that when a user asks us to delete their data (in this case: their annotation) we should be sure to actually delete that data from our systems, and this issue means that we sometimes fail to actually delete the data from Elasticsearch.

@seanh seanh self-assigned this Feb 16, 2024
seanh added a commit that referenced this issue Feb 19, 2024
Use the job queue to ensure that annotations get deleted from
Elasticsearch.

Fixes #7840
seanh added a commit that referenced this issue Feb 19, 2024
Use the job queue to ensure that annotations get deleted from
Elasticsearch.

Fixes #7840
seanh added a commit that referenced this issue Feb 26, 2024
Use the job queue to ensure that annotations get deleted from
Elasticsearch.

Fixes #7840
seanh added a commit that referenced this issue Feb 27, 2024
Use the job queue to ensure that annotations get deleted from
Elasticsearch.

Fixes #7840
seanh added a commit that referenced this issue Feb 28, 2024
Use the job queue to ensure that annotations get deleted from
Elasticsearch.

Fixes #7840
seanh added a commit that referenced this issue Feb 28, 2024
Use the job queue to ensure that annotations get deleted from
Elasticsearch.

Fixes #7840
seanh added a commit that referenced this issue Feb 28, 2024
Use the job queue to ensure that annotations get deleted from
Elasticsearch.

Fixes #7840
seanh added a commit that referenced this issue Feb 28, 2024
Use the job queue to ensure that annotations get deleted from
Elasticsearch.

Fixes #7840
seanh added a commit that referenced this issue Feb 29, 2024
Use the job queue to ensure that annotations get deleted from
Elasticsearch.

Fixes #7840
seanh added a commit that referenced this issue Feb 29, 2024
Use the job queue to ensure that annotations get deleted from
Elasticsearch.

Fixes #7840
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants