[ 
https://issues.apache.org/jira/browse/GEODE-8667?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17245588#comment-17245588
 ] 

ASF GitHub Bot commented on GEODE-8667:
---------------------------------------

jchen21 commented on pull request #5689:
URL: https://github.com/apache/geode/pull/5689#issuecomment-740265251


   > ```
   > 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.
   > ```
   
   I don't think we should avoid creating krf during offline compaction. Your 
proposed fix avoid creating the krf file during offline compaction. After the 
compaction, the original non-compacted krf is also deleted. Without a krf, 
there is a significant impact on the performance of persistent recovery. 


----------------------------------------------------------------
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:
us...@infra.apache.org


> Duplicate online Oplog compaction after offline Oplog compaction
> ----------------------------------------------------------------
>
>                 Key: GEODE-8667
>                 URL: https://issues.apache.org/jira/browse/GEODE-8667
>             Project: Geode
>          Issue Type: Bug
>            Reporter: Jianxia Chen
>            Assignee: Jianxia Chen
>            Priority: Major
>              Labels: GeodeOperationAPI, pull-request-available
>
> Use `compact offline-disk-store` command to compact the Oplogs offline. 
> Then restart the servers. 
> The logs show OplogCompactor thread is compacting Oplogs again when 
> restarting the servers, even though the Oplogs were just compacted offline. 
> For example:
> ```
> [info 2020/10/27 16:32:22.534 PDT <Idle OplogCompactor1> tid=0x35] Recovered 
> values for disk store DEFAULT with unique id 
> 76393d3c-dd10-4b89-b655-821d37631774
> [info 2020/10/27 16:32:22.535 PDT <OplogCompactor DEFAULT for oplog oplog#2> 
> tid=0x35] OplogCompactor for DEFAULT compaction oplog id(s): oplog#2
> [info 2020/10/27 16:32:22.537 PDT <OplogCompactor DEFAULT for oplog oplog#2> 
> tid=0x35] compaction did 2 creates and updates in 2 ms
> [info 2020/10/27 16:32:22.537 PDT <Oplog Delete Task1> tid=0x36] Deleted 
> oplog#2 crf for disk store DEFAULT.
> [info 2020/10/27 16:32:22.538 PDT <Oplog Delete Task1> tid=0x36] Deleted 
> oplog#2 krf for disk store DEFAULT.
> [info 2020/10/27 16:32:22.538 PDT <Oplog Delete Task1> tid=0x36] Deleted 
> oplog#2 drf for disk store DEFAULT.
> ```



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to