You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@asterixdb.apache.org by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu> on 2022/02/14 23:54:28 UTC

Change in asterixdb[neo]: [NO ISSUE][OTH] Ensure no failures during transaction completion

From Murtadha Hubail <mh...@apache.org>:

Murtadha Hubail has uploaded this change for review. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/15288 )


Change subject: [NO ISSUE][OTH] Ensure no failures during transaction completion
......................................................................

[NO ISSUE][OTH] Ensure no failures during transaction completion

- user model changes: no
- storage format changes: no
- interface changes: no

Details:

- When completing a transaction, only untouch an index
  if it was successfully touched at the beginning of the
  transaction.
- Log when an expected index is not found and throw an
  exception to avoid an NPE.

Change-Id: Ie0d4879630ae302485d595060dd87a896d151307
---
M asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
1 file changed, 15 insertions(+), 2 deletions(-)



  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/88/15288/1

diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
index f8a81e4..5964bb4 100644
--- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
+++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
@@ -25,9 +25,12 @@
 import org.apache.hyracks.storage.am.lsm.common.api.LSMOperationType;
 import org.apache.hyracks.storage.common.IModificationOperationCallback;
 import org.apache.hyracks.storage.common.ISearchOperationCallback;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 public class BaseOperationTracker implements ITransactionOperationTracker {
 
+    private static final Logger LOGGER = LogManager.getLogger();
     protected final int datasetID;
     protected final DatasetInfo dsInfo;
 
@@ -67,13 +70,23 @@
          * from being evicted/dropped until the transaction completes
          */
         dsInfo.touch();
-        dsInfo.getIndexes().get(resourceId).touch();
+        IndexInfo indexInfo = dsInfo.getIndexes().get(resourceId);
+        if (indexInfo == null) {
+            LOGGER.error("could not find resource id {} in dataset {}; registered indexes {}", resourceId, dsInfo,
+                    dsInfo.getIndexes());
+            throw new IllegalStateException("could not find resource id " + resourceId + " in dataset " + dsInfo);
+        }
+        indexInfo.touch();
     }
 
     @Override
     public void afterTransaction(long resourceId) {
         dsInfo.untouch();
-        dsInfo.getIndexes().get(resourceId).untouch();
+        IndexInfo indexInfo = dsInfo.getIndexes().get(resourceId);
+        if (indexInfo != null) {
+            // only untouch if the touch in beforeTransaction succeeded
+            indexInfo.untouch();
+        }
     }
 
     public DatasetInfo getDatasetInfo() {

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/15288
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: neo
Gerrit-Change-Id: Ie0d4879630ae302485d595060dd87a896d151307
Gerrit-Change-Number: 15288
Gerrit-PatchSet: 1
Gerrit-Owner: Murtadha Hubail <mh...@apache.org>
Gerrit-MessageType: newchange

Change in asterixdb[neo]: [NO ISSUE][OTH] Ensure no failures during transaction completion

Posted by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>.
From Jenkins <je...@fulliautomatix.ics.uci.edu>:

Jenkins has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/15288 )

Change subject: [NO ISSUE][OTH] Ensure no failures during transaction completion
......................................................................


Patch Set 1: Integration-Tests+1

Integration Tests Successful

https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-integration-tests/12969/ : SUCCESS


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/15288
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: neo
Gerrit-Change-Id: Ie0d4879630ae302485d595060dd87a896d151307
Gerrit-Change-Number: 15288
Gerrit-PatchSet: 1
Gerrit-Owner: Murtadha Hubail <mh...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Comment-Date: Tue, 15 Feb 2022 11:51:11 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in asterixdb[neo]: [NO ISSUE][OTH] Ensure no failures during transaction completion

Posted by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>.
From Murtadha Hubail <mh...@apache.org>:

Murtadha Hubail has uploaded this change for review. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/15288 )


Change subject: [NO ISSUE][OTH] Ensure no failures during transaction completion
......................................................................

[NO ISSUE][OTH] Ensure no failures during transaction completion

- user model changes: no
- storage format changes: no
- interface changes: no

Details:

- When completing a transaction, only untouch an index
  if it was successfully touched at the beginning of the
  transaction.
- Log when an expected index is not found and throw an
  exception to avoid an NPE.

Change-Id: Ie0d4879630ae302485d595060dd87a896d151307
---
M asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
1 file changed, 15 insertions(+), 2 deletions(-)



  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/88/15288/1

diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
index f8a81e4..5964bb4 100644
--- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
+++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
@@ -25,9 +25,12 @@
 import org.apache.hyracks.storage.am.lsm.common.api.LSMOperationType;
 import org.apache.hyracks.storage.common.IModificationOperationCallback;
 import org.apache.hyracks.storage.common.ISearchOperationCallback;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 public class BaseOperationTracker implements ITransactionOperationTracker {
 
+    private static final Logger LOGGER = LogManager.getLogger();
     protected final int datasetID;
     protected final DatasetInfo dsInfo;
 
@@ -67,13 +70,23 @@
          * from being evicted/dropped until the transaction completes
          */
         dsInfo.touch();
-        dsInfo.getIndexes().get(resourceId).touch();
+        IndexInfo indexInfo = dsInfo.getIndexes().get(resourceId);
+        if (indexInfo == null) {
+            LOGGER.error("could not find resource id {} in dataset {}; registered indexes {}", resourceId, dsInfo,
+                    dsInfo.getIndexes());
+            throw new IllegalStateException("could not find resource id " + resourceId + " in dataset " + dsInfo);
+        }
+        indexInfo.touch();
     }
 
     @Override
     public void afterTransaction(long resourceId) {
         dsInfo.untouch();
-        dsInfo.getIndexes().get(resourceId).untouch();
+        IndexInfo indexInfo = dsInfo.getIndexes().get(resourceId);
+        if (indexInfo != null) {
+            // only untouch if the touch in beforeTransaction succeeded
+            indexInfo.untouch();
+        }
     }
 
     public DatasetInfo getDatasetInfo() {

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/15288
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: neo
Gerrit-Change-Id: Ie0d4879630ae302485d595060dd87a896d151307
Gerrit-Change-Number: 15288
Gerrit-PatchSet: 1
Gerrit-Owner: Murtadha Hubail <mh...@apache.org>
Gerrit-MessageType: newchange

Change in asterixdb[neo]: [NO ISSUE][OTH] Ensure no failures during transaction completion

Posted by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>.
From Murtadha Hubail <mh...@apache.org>:

Murtadha Hubail has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/15288 )

Change subject: [NO ISSUE][OTH] Ensure no failures during transaction completion
......................................................................


Patch Set 1: Code-Review+1


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/15288
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: neo
Gerrit-Change-Id: Ie0d4879630ae302485d595060dd87a896d151307
Gerrit-Change-Number: 15288
Gerrit-PatchSet: 1
Gerrit-Owner: Murtadha Hubail <mh...@apache.org>
Gerrit-Reviewer: Ali Alsuliman <al...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-Reviewer: Murtadha Hubail <mh...@apache.org>
Gerrit-Comment-Date: Tue, 15 Feb 2022 13:24:40 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in asterixdb[neo]: [NO ISSUE][OTH] Ensure no failures during transaction completion

Posted by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>.
From Murtadha Hubail <mh...@apache.org>:

Murtadha Hubail has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/15288 )

Change subject: [NO ISSUE][OTH] Ensure no failures during transaction completion
......................................................................

[NO ISSUE][OTH] Ensure no failures during transaction completion

- user model changes: no
- storage format changes: no
- interface changes: no

Details:

- When completing a transaction, only untouch an index
  if it was successfully touched at the beginning of the
  transaction.
- Log when an expected index is not found and throw an
  exception to avoid an NPE.

Change-Id: Ie0d4879630ae302485d595060dd87a896d151307
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/15288
Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
Reviewed-by: Murtadha Hubail <mh...@apache.org>
Reviewed-by: Ali Alsuliman <al...@gmail.com>
Tested-by: Murtadha Hubail <mh...@apache.org>
Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
---
M asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
1 file changed, 15 insertions(+), 2 deletions(-)

Approvals:
  Murtadha Hubail: Looks good to me, but someone else must approve; Verified
  Ali Alsuliman: Looks good to me, approved
  Jenkins: Verified; Verified



diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
index f8a81e4..5964bb4 100644
--- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
+++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
@@ -25,9 +25,12 @@
 import org.apache.hyracks.storage.am.lsm.common.api.LSMOperationType;
 import org.apache.hyracks.storage.common.IModificationOperationCallback;
 import org.apache.hyracks.storage.common.ISearchOperationCallback;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 public class BaseOperationTracker implements ITransactionOperationTracker {
 
+    private static final Logger LOGGER = LogManager.getLogger();
     protected final int datasetID;
     protected final DatasetInfo dsInfo;
 
@@ -67,13 +70,23 @@
          * from being evicted/dropped until the transaction completes
          */
         dsInfo.touch();
-        dsInfo.getIndexes().get(resourceId).touch();
+        IndexInfo indexInfo = dsInfo.getIndexes().get(resourceId);
+        if (indexInfo == null) {
+            LOGGER.error("could not find resource id {} in dataset {}; registered indexes {}", resourceId, dsInfo,
+                    dsInfo.getIndexes());
+            throw new IllegalStateException("could not find resource id " + resourceId + " in dataset " + dsInfo);
+        }
+        indexInfo.touch();
     }
 
     @Override
     public void afterTransaction(long resourceId) {
         dsInfo.untouch();
-        dsInfo.getIndexes().get(resourceId).untouch();
+        IndexInfo indexInfo = dsInfo.getIndexes().get(resourceId);
+        if (indexInfo != null) {
+            // only untouch if the touch in beforeTransaction succeeded
+            indexInfo.untouch();
+        }
     }
 
     public DatasetInfo getDatasetInfo() {

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/15288
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: neo
Gerrit-Change-Id: Ie0d4879630ae302485d595060dd87a896d151307
Gerrit-Change-Number: 15288
Gerrit-PatchSet: 2
Gerrit-Owner: Murtadha Hubail <mh...@apache.org>
Gerrit-Reviewer: Ali Alsuliman <al...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-Reviewer: Murtadha Hubail <mh...@apache.org>
Gerrit-MessageType: merged