dschneider-pivotal commented on a change in pull request #7145:
URL: https://github.com/apache/geode/pull/7145#discussion_r763186545
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java
##########
@@ -1303,16 +1305,18 @@ void close(DiskRegion dr) {
// while a krf is being created can not close a region
lockCompactor();
try {
- addUnrecoveredRegion(dr.getId());
- DiskRegionInfo dri = getDRI(dr);
- if (dri != null) {
- long clearCount = dri.clear(null);
- if (clearCount != 0) {
- totalLiveCount.addAndGet(-clearCount);
- // no need to call handleNoLiveValues because we now have an
- // unrecovered region.
+ if (!isDrfOnly()) {
Review comment:
I think you have addressed most of my concerns. I'm still worried about
the following corner case if we don't add a drf only oplog to the unrecovered
region count.
What happens if you have a region that still has live data in an old crf
oplog and has only deletes in a newer drf oplog. That newer oplog will be kept
alive by the older crf so the newer oplog will be marked as drf only. Then
someone calls "Region.close" on that region. This causes the persistent data to
be kept on disk but removed from memory. Then that same member turns around and
recreates the region it closed. If the close did not remember that drf only
oplog had an "unrecovered region" for the drf only one then I'm not sure the
recovery will work when the region is recreated. Since it does not reread the
disk store files (if, crf, drf, krf) in their entirety (like we do during a
normal startup) enough info needs to be kept in memory so that we know that drf
only oplog has some info about the region we are recreating. I think this
association on the recreate happens in this "checkForRecoverableRegion which
will call dri.testAndSetRecovered which will associate the dri with it
s DiskRegionView".
Does a test exist for this use case (close + recreate with drf only oplog)?
If not if you add one you might find that drf only oplogs are handled in some
other way during recreate without them being in the unrecoveredRegionCount.
--
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]
