rishabhdaim commented on code in PR #3137:
URL: https://github.com/apache/jackrabbit-oak/pull/3137#discussion_r4091981236
##########
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:
@jsedding
1. We are not updating gc.log after cleanup on the standBy side. (Cleanup is
enabled by default).
2. And this is problematic since a stale gc.log would cause SNFE when
referenced segments don't exist.
R.e the change (where you have added a comment). I am not sure whether it is
correct or if it might affect some other flows. But @Shivam-Agg tested and
confirmed that without that change, we are logging twice the entries in gc.log.
--
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]
