rishabhdaim commented on code in PR #3137:
URL: https://github.com/apache/jackrabbit-oak/pull/3137#discussion_r4092581745
##########
oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/GCJournal.java:
##########
@@ -68,7 +68,15 @@ public synchronized void persist(long reclaimedSize, long
repoSize,
@NotNull GCGeneration gcGeneration, long nodes, @NotNull String
root
) {
GCJournalEntry current = read();
- if (current.getGcGeneration().equals(gcGeneration)) {
+ GCGeneration currentGeneration = current.getGcGeneration();
+ // Compare generation and fullGeneration only, not isCompacted: the
on-disk journal
+ // format never persists isCompacted (GCJournalEntry.fromString always
deserializes it
+ // as false), so comparing the full GCGeneration would wrongly treat
every persist() of
+ // an already-journaled, still-compacted generation as new after a
restart (e.g. a
+ // standalone cleanup() on a store re-opened with an already-compacted
head), causing a
+ // duplicate entry to be written on every such restart.
+ if (currentGeneration.getGeneration() == gcGeneration.getGeneration()
+ && currentGeneration.getFullGeneration() ==
gcGeneration.getFullGeneration()) {
Review Comment:
>I don't understand why you write
We are not updating gc.log after cleanup on the standBy side.
(Cleanup is enabled by default).
And this is problematic since a stale gc.log would cause SNFE when
referenced segments don't exist.
>Isn't that exactly what this PR is meant to resolve?
Yes, I was summarizing the changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
