gesterzhou commented on pull request #5689:
URL: https://github.com/apache/geode/pull/5689#issuecomment-739465537
I read your new fix and have different opinion. The root cause is the new
oplog (i.e. #2) created during offline compaction wrote a krf with
totalCount==0.
You fix is to still read the krf and when found it's totalCount is 0, change
it to be totalLiveCount. This is just a patch fix. The real issue is: we should
NOT create such a krf with totalCount==0. Actually, when offline compaction, it
is not allowed to create krf.
Here is the better fix for this issue:
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java
index f6daa3a1d6..0a31af9386 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java
@@ -582,7 +582,7 @@ public class Oplog implements CompactableOplog,
Flushable {
createDrf(null);
createCrf(null);
// open krf for offline compaction
- if (getParent().isOfflineCompacting()) {
+ if (getParent().isOfflineCompacting() && couldHaveKrf()) {
krfFileCreate();
}
} catch (Exception ex) {
@@ -3980,7 +3980,7 @@ public class Oplog implements CompactableOplog,
Flushable {
} catch (IOException ignore) {
}
- if (this.krf.f.exists()) {
+ if (this.krf.f != null && this.krf.f.exists()) {
this.krf.f.delete();
}
}
@@ -5789,7 +5789,7 @@ public class Oplog implements CompactableOplog,
Flushable {
if (((rv / (double) rvHWMtmp) * 100) <=
parent.getCompactionThreshold()) {
return true;
}
- } else if (this.totalLiveCount.get() <= 0) {
+ } else {
return true;
}
I don't know who introduced "if (getParent().isOfflineCompacting()" here. To
be conservative, I still keep this check. Maybe we can remove the 3 lines. But
I want to be conservative for now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
