Feature/lxl 4524 #1686
Open
Feature/lxl 4524 #1686
Conversation
This file contains hidden or 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
jannistsiroyannis
requested review from
andersju,
kaipoykio,
kwahlin and
olovy
andersju
reviewed
Jan 28, 2026
Member
andersju
left a comment
andersju
left a comment
There was a problem hiding this comment.
Nice, LGTM but see comments! Additionally:
- I suggest moving cutoff time (line 71), batch size (73), and runtime (210) to constants at the top. Maybe also the checkpoint threshold (155).
- logger.info walPath on trigger, or at least archiveRoot on startup?
housekeeping/src/main/groovy/whelk/housekeeping/HistoryArchiver.java
Outdated
Show resolved
Hide resolved
Contributor
Author
Thank you! Excellent feedback! ⭐ I agree with and have fixed all of the issues, except the moving of values to the top. I find this to be useful only if the values are used more than once, otherwise i prefer them where they are used. |
andersju
approved these changes
Jan 28, 2026
olovy
requested changes
Jan 30, 2026
Contributor
olovy
left a comment
olovy
left a comment
There was a problem hiding this comment.
Requested change: We shouldn't delete the tombstone in lddb. We should be able to tell that the resource has existed even though we delete the history (possibly including the last version in lddb).
Contributor
|
Is the added complexity of generating diffs worth it now that we archive the history to disk instead of trying to compact the history table? |
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.
Adds history archiving (of deleted records) to librisXL. A change to secret.properties (setting a base path for the archive) is necessary. Until it is set, this does nothing (but also isn't destructive in any way).
The history archive consists of gzipped json-lines files. Each around 100Mb in size and named/tagged with the time of archiving. These can be easily searched for individual records, typically using "zcat | grep". Each line consists of an 'original' version and a set of diffs for each following version.