-
Notifications
You must be signed in to change notification settings - Fork 41
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
tasks: add margin time window for search to purge deleted documents #244
Merged
zzacharo
merged 1 commit into
inveniosoftware:master
from
zzacharo:add-search-gc-deleted
Aug 25, 2023
Merged
tasks: add margin time window for search to purge deleted documents #244
zzacharo
merged 1 commit into
inveniosoftware:master
from
zzacharo:add-search-gc-deleted
Aug 25, 2023
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
zzacharo
force-pushed
the
add-search-gc-deleted
branch
from
August 24, 2023 15:48
89cc9da
to
37b10f2
Compare
zzacharo
force-pushed
the
add-search-gc-deleted
branch
from
August 24, 2023 16:16
37b10f2
to
86e455d
Compare
ntarocco
approved these changes
Aug 25, 2023
Comment on lines
239
to
241
:param int search_gc_deletes: default time search is keeping deleted documents for | ||
control of concurrent operations. For more information see: | ||
https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-delete.html#delete-versioning |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested change
:param int search_gc_deletes: default time search is keeping deleted documents for | |
control of concurrent operations. For more information see: | |
https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-delete.html#delete-versioning | |
:param int search_gc_deletes: time in seconds, corresponding to the search cluster | |
setting `index.gc_deletes` (see | |
https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-delete.html#delete-versioning), | |
default to 60 seconds. Search cluster caches deleted documents for `index.gc_deletes` seconds. |
Comment on lines
236
to
239
"""Clean up (hard delete) all the soft deleted drafts. | ||
|
||
The drafts in the last timedelta span of time won't be deleted. | ||
:param int search_gc_deletes: default time search is keeping deleted documents for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested change
"""Clean up (hard delete) all the soft deleted drafts. | |
The drafts in the last timedelta span of time won't be deleted. | |
:param int search_gc_deletes: default time search is keeping deleted documents for | |
"""Clean up (hard delete) all the soft deleted drafts. | |
The soft-deleted drafts in the last timedelta span of time won't be deleted, including | |
`search_gc_deletes` seconds timedelta: this ensures that only drafts fully removed | |
from the search cluster can be hard-deleted (e.g. when `td` is very short), avoiding | |
search conflicts. | |
:param int search_gc_deletes: default time search is keeping deleted documents for |
zzacharo
force-pushed
the
add-search-gc-deleted
branch
from
August 25, 2023 07:53
86e455d
to
9e51951
Compare
zzacharo
force-pushed
the
add-search-gc-deleted
branch
from
August 25, 2023 08:17
9e51951
to
e9f6fb8
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.
Soft deleted records are being kept in Opensearch for 60 seconds by default. That is happening for control of concurrent processes. In our case, when a soft deleted draft is cleaned up but it happened to be soft deleted in less than the time window that Opensearch finally cleans it up, then editing a new record is resulting in version conflict.
This PR adds a new kwarg to deduct the default deletion time and it can be passed as a parameter in case someone has configured the corresponding setting in a different value.