You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "swamirishi (via GitHub)" <gi...@apache.org> on 2024/09/11 16:19:00 UTC

[PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

swamirishi opened a new pull request, #7188:
URL: https://github.com/apache/ozone/pull/7188

   ## What changes were proposed in this pull request?
   Since purgeSnapshot transaction on a standalone basis is an idempotent operation. Once the directory gets deleted the previous transactions that have been performed on the snapshotted rocksdb would start failing on replay since those transactions have not been committed but the  directory could have been partially deleted/ fully deleted. This could also lead to inconsistencies in the DB  reads from the purged rocksdb if operations are not performed consciously. 
   Creating a separate rocksdb batch in ozone manager double buffer will ensure all transactions before have been flushed to the disk before purging the rocksdb, ensuring the previous ratis transactions will never get replayed.
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/HDDS-11453
   
   ## How was this patch tested?
   Changed existing unit tests.
   


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #7188:
URL: https://github.com/apache/ozone/pull/7188#issuecomment-2346321603

   @hemantk-12 please take a look at the patch again. There was a findbugs failure. I added an annotation in OMDoubleBuffer class to suppress the warnings.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #7188:
URL: https://github.com/apache/ozone/pull/7188#discussion_r1755772997


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerDoubleBuffer.java:
##########
@@ -437,17 +448,22 @@ private String addToBatch(Queue<Entry> buffer, BatchOperation batchOperation) {
    */
   private List<Queue<Entry>> splitReadyBufferAtCreateSnapshot() {
     final List<Queue<Entry>> response = new ArrayList<>();
-
+    final Set<OzoneManagerProtocolProtos.Type> standaloneBatchCmdTypes = ImmutableSet.of(
+        OzoneManagerProtocolProtos.Type.SnapshotPurge, OzoneManagerProtocolProtos.Type.CreateSnapshot);
+    final List<Function<OMResponse, Boolean>> standaloneBatchConditions =
+        ImmutableList.of(OMResponse::hasCreateSnapshotResponse,

Review Comment:
   this was the previous logic. I can remove it.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #7188:
URL: https://github.com/apache/ozone/pull/7188#discussion_r1757388443


##########
hadoop-ozone/ozone-manager/dev-support/findbugsExcludeFile.xml:
##########
@@ -89,4 +89,9 @@
     <Class name="org.apache.hadoop.ozone.om.upgrade.TestOMLayoutFeatureAspect"/>
     <Bug pattern="BC_IMPOSSIBLE_CAST" />
   </Match>
+  <Match>
+    <Class name="org.apache.hadoop.ozone.om.ratis.OzoneManagerDoubleBuffer" />
+    <Field name="readyBuffer" />
+    <Bug pattern="IS2_INCONSISTENT_SYNC" />
+  </Match>

Review Comment:
   yeah I agree about that. Let me make the method synchronized



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #7188:
URL: https://github.com/apache/ozone/pull/7188#discussion_r1757338835


##########
hadoop-ozone/ozone-manager/dev-support/findbugsExcludeFile.xml:
##########
@@ -89,4 +89,9 @@
     <Class name="org.apache.hadoop.ozone.om.upgrade.TestOMLayoutFeatureAspect"/>
     <Bug pattern="BC_IMPOSSIBLE_CAST" />
   </Match>
+  <Match>
+    <Class name="org.apache.hadoop.ozone.om.ratis.OzoneManagerDoubleBuffer" />
+    <Field name="readyBuffer" />
+    <Bug pattern="IS2_INCONSISTENT_SYNC" />
+  </Match>

Review Comment:
   The `splitReadyBufferAtCreateSnapshot()` just has a simple loop.  Adding `synchronized ` won't affect the performance   On the other hand, suppressing `IS2_INCONSISTENT_SYNC` will hide the real bug in the future.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #7188:
URL: https://github.com/apache/ozone/pull/7188#discussion_r1755766115


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerDoubleBuffer.java:
##########
@@ -437,17 +448,22 @@ private String addToBatch(Queue<Entry> buffer, BatchOperation batchOperation) {
    */
   private List<Queue<Entry>> splitReadyBufferAtCreateSnapshot() {
     final List<Queue<Entry>> response = new ArrayList<>();
-
+    final Set<OzoneManagerProtocolProtos.Type> standaloneBatchCmdTypes = ImmutableSet.of(
+        OzoneManagerProtocolProtos.Type.SnapshotPurge, OzoneManagerProtocolProtos.Type.CreateSnapshot);
+    final List<Function<OMResponse, Boolean>> standaloneBatchConditions =
+        ImmutableList.of(OMResponse::hasCreateSnapshotResponse,

Review Comment:
   Why do we need to check `OMResponse::hasCreateSnapshotResponse`? Can't we just rely on `CmdType`?



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerDoubleBuffer.java:
##########
@@ -437,17 +448,22 @@ private String addToBatch(Queue<Entry> buffer, BatchOperation batchOperation) {
    */
   private List<Queue<Entry>> splitReadyBufferAtCreateSnapshot() {
     final List<Queue<Entry>> response = new ArrayList<>();
-
+    final Set<OzoneManagerProtocolProtos.Type> standaloneBatchCmdTypes = ImmutableSet.of(
+        OzoneManagerProtocolProtos.Type.SnapshotPurge, OzoneManagerProtocolProtos.Type.CreateSnapshot);
+    final List<Function<OMResponse, Boolean>> standaloneBatchConditions =
+        ImmutableList.of(OMResponse::hasCreateSnapshotResponse,
+        (omResponse) -> standaloneBatchCmdTypes.contains(omResponse.getCmdType()));
     OMResponse previousOmResponse = null;
     for (final Entry entry : readyBuffer) {
+      OMResponse prevResponse = previousOmResponse;
       OMResponse omResponse = entry.getResponse().getOMResponse();
       // New queue gets created in three conditions:
       // 1. It is first element in the response,
       // 2. Current request is createSnapshot request.
       // 3. Previous request was createSnapshot request.

Review Comment:
   Please up date this comment.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #7188:
URL: https://github.com/apache/ozone/pull/7188#discussion_r1756987726


##########
pom.xml:
##########
@@ -1312,6 +1313,11 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs
         <artifactId>nimbus-jose-jwt</artifactId>
         <version>${com.nimbusds.nimbus-jose-jwt.version}</version>
       </dependency>
+      <dependency>
+        <groupId>com.github.spotbugs</groupId>
+        <artifactId>spotbugs-annotations</artifactId>
+        <version>${spotbugs.annotation.version}</version>
+      </dependency>

Review Comment:
   Added the field to be excluded on the excludeFile



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #7188:
URL: https://github.com/apache/ozone/pull/7188#discussion_r1757333412


##########
hadoop-ozone/ozone-manager/dev-support/findbugsExcludeFile.xml:
##########
@@ -89,4 +89,9 @@
     <Class name="org.apache.hadoop.ozone.om.upgrade.TestOMLayoutFeatureAspect"/>
     <Bug pattern="BC_IMPOSSIBLE_CAST" />
   </Match>
+  <Match>
+    <Class name="org.apache.hadoop.ozone.om.ratis.OzoneManagerDoubleBuffer" />
+    <Field name="readyBuffer" />
+    <Bug pattern="IS2_INCONSISTENT_SYNC" />
+  </Match>

Review Comment:
   We don't need an object lock on double buffer. That would impact the entire write flow. Only flushing daemon uses this readBuffer. Active writes happen to current buffer we shouldn't be blocking that. Object lock will block that. Ideally we could have implemented individual locks than depending on the entire object lock. This would be more optimized.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #7188:
URL: https://github.com/apache/ozone/pull/7188#discussion_r1756912289


##########
pom.xml:
##########
@@ -1312,6 +1313,11 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs
         <artifactId>nimbus-jose-jwt</artifactId>
         <version>${com.nimbusds.nimbus-jose-jwt.version}</version>
       </dependency>
+      <dependency>
+        <groupId>com.github.spotbugs</groupId>
+        <artifactId>spotbugs-annotations</artifactId>
+        <version>${spotbugs.annotation.version}</version>
+      </dependency>

Review Comment:
   `spotbugs-annotations` has LGPL license, we cannot use it.  Suppress the issue in `hadoop-ozone/ozone-manager/dev-support/findbugsExcludeFile.xml` (or fix it if you can).
   
   ```
        (GNU LESSER GENERAL PUBLIC LICENSE, Version 2.1) SpotBugs Annotations (com.github.spotbugs:spotbugs-annotations:3.1.12 - https://spotbugs.github.io/)
   ```
   
   https://github.com/apache/ozone/actions/runs/10831822593/job/30055799681?pr=7188#step:6:9



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #7188:
URL: https://github.com/apache/ozone/pull/7188#discussion_r1757533025


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerDoubleBuffer.java:
##########
@@ -437,17 +453,21 @@ private String addToBatch(Queue<Entry> buffer, BatchOperation batchOperation) {
    */
   private List<Queue<Entry>> splitReadyBufferAtCreateSnapshot() {
     final List<Queue<Entry>> response = new ArrayList<>();
-
+    final Set<OzoneManagerProtocolProtos.Type> standaloneBatchCmdTypes = ImmutableSet.of(
+        OzoneManagerProtocolProtos.Type.SnapshotPurge, OzoneManagerProtocolProtos.Type.CreateSnapshot);
+    final List<Function<OMResponse, Boolean>> standaloneBatchConditions =
+        ImmutableList.of((omResponse) -> standaloneBatchCmdTypes.contains(omResponse.getCmdType()));

Review Comment:
   done



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #7188:
URL: https://github.com/apache/ozone/pull/7188#discussion_r1755878436


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerDoubleBuffer.java:
##########
@@ -437,17 +448,22 @@ private String addToBatch(Queue<Entry> buffer, BatchOperation batchOperation) {
    */
   private List<Queue<Entry>> splitReadyBufferAtCreateSnapshot() {
     final List<Queue<Entry>> response = new ArrayList<>();
-
+    final Set<OzoneManagerProtocolProtos.Type> standaloneBatchCmdTypes = ImmutableSet.of(
+        OzoneManagerProtocolProtos.Type.SnapshotPurge, OzoneManagerProtocolProtos.Type.CreateSnapshot);
+    final List<Function<OMResponse, Boolean>> standaloneBatchConditions =
+        ImmutableList.of(OMResponse::hasCreateSnapshotResponse,

Review Comment:
   done



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerDoubleBuffer.java:
##########
@@ -437,17 +448,22 @@ private String addToBatch(Queue<Entry> buffer, BatchOperation batchOperation) {
    */
   private List<Queue<Entry>> splitReadyBufferAtCreateSnapshot() {
     final List<Queue<Entry>> response = new ArrayList<>();
-
+    final Set<OzoneManagerProtocolProtos.Type> standaloneBatchCmdTypes = ImmutableSet.of(
+        OzoneManagerProtocolProtos.Type.SnapshotPurge, OzoneManagerProtocolProtos.Type.CreateSnapshot);
+    final List<Function<OMResponse, Boolean>> standaloneBatchConditions =
+        ImmutableList.of(OMResponse::hasCreateSnapshotResponse,
+        (omResponse) -> standaloneBatchCmdTypes.contains(omResponse.getCmdType()));
     OMResponse previousOmResponse = null;
     for (final Entry entry : readyBuffer) {
+      OMResponse prevResponse = previousOmResponse;
       OMResponse omResponse = entry.getResponse().getOMResponse();
       // New queue gets created in three conditions:
       // 1. It is first element in the response,
       // 2. Current request is createSnapshot request.
       // 3. Previous request was createSnapshot request.

Review Comment:
   done



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #7188:
URL: https://github.com/apache/ozone/pull/7188#discussion_r1757353630


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerDoubleBuffer.java:
##########
@@ -437,17 +453,21 @@ private String addToBatch(Queue<Entry> buffer, BatchOperation batchOperation) {
    */
   private List<Queue<Entry>> splitReadyBufferAtCreateSnapshot() {
     final List<Queue<Entry>> response = new ArrayList<>();
-
+    final Set<OzoneManagerProtocolProtos.Type> standaloneBatchCmdTypes = ImmutableSet.of(
+        OzoneManagerProtocolProtos.Type.SnapshotPurge, OzoneManagerProtocolProtos.Type.CreateSnapshot);
+    final List<Function<OMResponse, Boolean>> standaloneBatchConditions =
+        ImmutableList.of((omResponse) -> standaloneBatchCmdTypes.contains(omResponse.getCmdType()));

Review Comment:
   BTW, let's add a new `isStandaloneBatchCmdTypes(..)` method.  The code becomes much easier to understand.
   ```java
     private synchronized List<Queue<Entry>> splitReadyBufferAtCreateSnapshot() {
       final List<Queue<Entry>> response = new ArrayList<>();
       OMResponse prevResponse = null;
       for (final Entry entry : readyBuffer) {
         OMResponse omResponse = entry.getResponse().getOMResponse();
         // New queue gets created in three conditions:
         // 1. It is first element in the response,
         // 2. Current request is createSnapshot/purgeSnapshot request.
         // 3. Previous request was createSnapshot/purgeSnapshot request.
         if (response.isEmpty() || isStandaloneBatchCmdTypes(omResponse) || isStandaloneBatchCmdTypes(prevResponse)) {
           response.add(new LinkedList<>());
         }
   
         response.get(response.size() - 1).add(entry);
         prevResponse = omResponse;
       }
   
       return response;
     }
   
     static boolean isStandaloneBatchCmdTypes(OMResponse response) {
       if (response == null) {
         return false;
       }
       final OzoneManagerProtocolProtos.Type type = response.getCmdType();
       return type == OzoneManagerProtocolProtos.Type.SnapshotPurge
           || type == OzoneManagerProtocolProtos.Type.CreateSnapshot;
     }
   ```



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #7188:
URL: https://github.com/apache/ozone/pull/7188#discussion_r1757318617


##########
hadoop-ozone/ozone-manager/dev-support/findbugsExcludeFile.xml:
##########
@@ -89,4 +89,9 @@
     <Class name="org.apache.hadoop.ozone.om.upgrade.TestOMLayoutFeatureAspect"/>
     <Bug pattern="BC_IMPOSSIBLE_CAST" />
   </Match>
+  <Match>
+    <Class name="org.apache.hadoop.ozone.om.ratis.OzoneManagerDoubleBuffer" />
+    <Field name="readyBuffer" />
+    <Bug pattern="IS2_INCONSISTENT_SYNC" />
+  </Match>

Review Comment:
   @swamirishi , How about adding `synchronized` to `splitReadyBufferAtCreateSnapshot()`?



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi merged PR #7188:
URL: https://github.com/apache/ozone/pull/7188


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #7188:
URL: https://github.com/apache/ozone/pull/7188#discussion_r1757388443


##########
hadoop-ozone/ozone-manager/dev-support/findbugsExcludeFile.xml:
##########
@@ -89,4 +89,9 @@
     <Class name="org.apache.hadoop.ozone.om.upgrade.TestOMLayoutFeatureAspect"/>
     <Bug pattern="BC_IMPOSSIBLE_CAST" />
   </Match>
+  <Match>
+    <Class name="org.apache.hadoop.ozone.om.ratis.OzoneManagerDoubleBuffer" />
+    <Field name="readyBuffer" />
+    <Bug pattern="IS2_INCONSISTENT_SYNC" />
+  </Match>

Review Comment:
   yeah I agree about that. Ok then let me make the method synchronized



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #7188:
URL: https://github.com/apache/ozone/pull/7188#discussion_r1756912289


##########
pom.xml:
##########
@@ -1312,6 +1313,11 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs
         <artifactId>nimbus-jose-jwt</artifactId>
         <version>${com.nimbusds.nimbus-jose-jwt.version}</version>
       </dependency>
+      <dependency>
+        <groupId>com.github.spotbugs</groupId>
+        <artifactId>spotbugs-annotations</artifactId>
+        <version>${spotbugs.annotation.version}</version>
+      </dependency>

Review Comment:
   `spotbugs-annotations` has LGPL license, we cannot use it.  Suppress the issue in `hadoop-ozone/ozone-manager/dev-support/findbugsExcludeFile.xml` (or fix it if you can).



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-11453. OmSnapshotPurge should be in a different ozone manager double buffer batch [ozone]

Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #7188:
URL: https://github.com/apache/ozone/pull/7188#issuecomment-2348871920

   Thank you for reviewing the PR @szetszwo @prashantpogde @aswinshakil @adoroszlai @hemantk-12 


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org