You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@asterixdb.apache.org by "Ian Maxon (Code Review)" <do...@asterixdb.incubator.apache.org> on 2016/11/02 03:16:29 UTC

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Ian Maxon has uploaded a new change for review.

  https://asterix-gerrit.ics.uci.edu/1331

Change subject: Fix for ASTERIXDB-1725
......................................................................

Fix for ASTERIXDB-1725

This is tricky, the best method I could think to fix this involved simply
putting the root page number in the metadata page rather than trying to
calculate it somehow going backwards from the end of the file.

I also fixed a bug discovered by Mike B where getLSNOffset was not
taking into consideration page header sizes.

Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
---
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IMetaDataPageManager.java
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexMetaDataFrame.java
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java
M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualMetaDataPageManager.java
6 files changed, 85 insertions(+), 21 deletions(-)


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

diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IMetaDataPageManager.java b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IMetaDataPageManager.java
index 3982a3a..5d87f99 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IMetaDataPageManager.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IMetaDataPageManager.java
@@ -172,4 +172,8 @@
     long getLSNOffset() throws HyracksDataException;
 
     public long getLastMarkerLSN() throws HyracksDataException;
+
+    void setRootPage(int rootPage) throws HyracksDataException;
+
+    int getRootPage() throws HyracksDataException;
 }
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexMetaDataFrame.java b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexMetaDataFrame.java
index d4fbaa2..21e918d 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexMetaDataFrame.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexMetaDataFrame.java
@@ -25,7 +25,7 @@
 
     //Storage version #. Change this if you alter any tree frame formats to stop
     // possible corruption from old versions reading new formats.
-    public static final int VERSION = 2;
+    public static final int VERSION = 3;
 
     public void initBuffer(byte level);
 
@@ -74,4 +74,8 @@
     public long getLastMarkerLSN();
 
     public void setLastMarkerLSN(long lsn);
+
+    void setRootPageNumber(int rootPage);
+
+    int getRootPageNumber();
 }
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java
index d4194c2..b35468f 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java
@@ -49,7 +49,8 @@
     public static final int LSN_OFFSET = ADDITIONAL_FILTERING_PAGE_OFFSET + 4; // 33
     private static final int LAST_MARKER_LSN_OFFSET = LSN_OFFSET + 8; // 41
     public static final int STORAGE_VERSION_OFFSET = LAST_MARKER_LSN_OFFSET + 4; //45
-    private static final int HEADER_END_OFFSET = LAST_MARKER_LSN_OFFSET + 4; //49
+    public static final int ROOT_PAGE_NUMBER = STORAGE_VERSION_OFFSET + 4; //49
+    private static final int HEADER_END_OFFSET = ROOT_PAGE_NUMBER + 4; // 53
 
     protected ICachedPage page = null;
     protected ByteBuffer buf = null;
@@ -126,6 +127,7 @@
         buf.putInt(NEXT_PAGE_OFFSET, -1);
         buf.putInt(ADDITIONAL_FILTERING_PAGE_OFFSET, -1);
         buf.putLong(LAST_MARKER_LSN_OFFSET, -1L);
+        buf.putInt(ROOT_PAGE_NUMBER,0);
         buf.putInt(STORAGE_VERSION_OFFSET, VERSION);
         setValid(false);
     }
@@ -192,4 +194,14 @@
     public void setLSMComponentFilterPageId(int filterPage) {
         buf.putInt(ADDITIONAL_FILTERING_PAGE_OFFSET, filterPage);
     }
+
+    @Override
+    public void setRootPageNumber(int rootPage) {
+        buf.putInt(ROOT_PAGE_NUMBER, rootPage);
+    }
+
+    @Override
+    public int getRootPageNumber() {
+        return buf.getInt(ROOT_PAGE_NUMBER);
+    }
 }
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java
index e7a1123..abe10df 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java
@@ -18,7 +18,6 @@
  */
 package org.apache.hyracks.storage.am.common.freepage;
 
-
 import org.apache.hyracks.api.exceptions.HyracksDataException;
 import org.apache.hyracks.storage.am.common.api.IMetaDataPageManager;
 import org.apache.hyracks.storage.am.common.api.ITreeIndexMetaDataFrame;
@@ -64,7 +63,7 @@
                 int newPage = metaFrame.getFreePage();
                 if (newPage < 0) {
                     throw new HyracksDataException(
-                              "Inconsistent Meta Page State. It has no space, but it also has no entries.");
+                            "Inconsistent Meta Page State. It has no space, but it also has no entries.");
                 }
 
                 ICachedPage newNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, newPage), false);
@@ -471,7 +470,8 @@
     public long getLSNOffset() throws HyracksDataException {
         int metadataPageNum = getFirstMetadataPage();
         if (metadataPageNum != IBufferCache.INVALID_PAGEID) {
-            return ((long)metadataPageNum * bufferCache.getPageSize()) + LIFOMetaDataFrame.LSN_OFFSET;
+            return ((long) metadataPageNum * (bufferCache.getPageSize() + BufferCache.RESERVED_HEADER_BYTES))
+                    + LIFOMetaDataFrame.LSN_OFFSET;
         }
         return IMetaDataPageManager.INVALID_LSN_OFFSET;
     }
@@ -496,4 +496,48 @@
             }
         }
     }
+
+    @Override
+    public void setRootPage(int rootPage) throws HyracksDataException{
+        ICachedPage metaNode;
+        if (!appendOnly) {
+            metaNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, getFirstMetadataPage()), false);
+        } else {
+            metaNode = confiscatedMetaNode;
+        }
+        ITreeIndexMetaDataFrame metaFrame = metaDataFrameFactory.createFrame();
+        metaNode.acquireWriteLatch();
+        try {
+            metaFrame.setPage(metaNode);
+            metaFrame.setRootPageNumber(rootPage);
+        } finally {
+            if (!appendOnly) {
+                metaNode.releaseWriteLatch(true);
+                bufferCache.unpin(metaNode);
+            } else {
+                metaNode.releaseWriteLatch(false);
+            }
+        }
+    }
+
+    @Override
+    public int getRootPage() throws HyracksDataException {
+        ICachedPage metaNode;
+        if (!appendOnly || confiscatedMetaNode == null) {
+            metaNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, getFirstMetadataPage()), false);
+        } else {
+            metaNode = confiscatedMetaNode;
+        }
+        ITreeIndexMetaDataFrame metaFrame = metaDataFrameFactory.createFrame();
+        metaNode.acquireReadLatch();
+        try {
+            metaFrame.setPage(metaNode);
+            return metaFrame.getRootPageNumber();
+        } finally {
+            metaNode.releaseReadLatch();
+            if (!appendOnly || confiscatedMetaNode == null) {
+                bufferCache.unpin(metaNode);
+            }
+        }
+    }
 }
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java
index 0dcfc90..5ac203e 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java
@@ -114,7 +114,6 @@
         }
 
         freePageManager.open(fileId);
-        setRootAndMetadataPages(appendOnly);
         if (!appendOnly) {
             initEmptyTree();
             freePageManager.close();
@@ -122,6 +121,7 @@
             this.appendOnly = true;
             initCachedMetadataPage();
         }
+        setRootPage(appendOnly);
         bufferCache.closeFile(fileId);
     }
 
@@ -140,25 +140,14 @@
         }
     }
 
-    private void setRootAndMetadataPages(boolean appendOnly) throws HyracksDataException {
+    private void setRootPage(boolean appendOnly) throws HyracksDataException {
         if (!appendOnly) {
             // regular or empty tree
             rootPage = 1;
             bulkloadLeafStart = 2;
         } else {
-            //the root page is either page n-2 (no filter) or n-3 (filter)
-            int numPages = bufferCache.getNumPagesOfFile(fileId);
-            if (numPages > MINIMAL_TREE_PAGE_COUNT) {
-                int filterPageId = freePageManager.getFilterPageId();
-                if (filterPageId > 0) {
-                    rootPage = numPages - MINIMAL_TREE_PAGE_COUNT_WITH_FILTER;
-                } else {
-                    rootPage = numPages - MINIMAL_TREE_PAGE_COUNT;
-                }
-            } else {
-                rootPage = 0;
-            }
-
+            //root page is stored in MD page
+            rootPage = freePageManager.getRootPage();
             //leaves start from the very beginning of the file.
             bulkloadLeafStart = 0;
         }
@@ -201,7 +190,7 @@
         } else {
             appendOnly = false;
         }
-        setRootAndMetadataPages(appendOnly);
+        setRootPage(appendOnly);
 
         // TODO: Should probably have some way to check that the tree is physically consistent
         // or that the file we just opened actually is a tree
@@ -424,6 +413,7 @@
 
                 }
             }
+            freePageManager.setRootPage(rootPage);
         }
 
         protected void addLevel() throws HyracksDataException {
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualMetaDataPageManager.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualMetaDataPageManager.java
index 158c68f..d6685af 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualMetaDataPageManager.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualMetaDataPageManager.java
@@ -184,4 +184,14 @@
         // Method doesn't make sense for this free page manager.
         return -1L;
     }
+
+    @Override
+    public void setRootPage(int rootPage) throws HyracksDataException {
+
+    }
+
+    @Override
+    public int getRootPage() throws HyracksDataException {
+        return 1;
+    }
 }

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 3: Integration-Tests+1

Integration Tests Successful

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

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 3
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Ian Maxon (Code Review)" <do...@asterixdb.incubator.apache.org>.
Ian Maxon has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 1:

(3 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1331/1/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java:

Line 130:         buf.putInt(ROOT_PAGE_NUMBER,0);
> Yeah, it does others of these, need to figure out why not this occurrence
Done


https://asterix-gerrit.ics.uci.edu/#/c/1331/1/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java:

Line 501:     public void setRootPage(int rootPage) throws HyracksDataException{
> +1
Done


https://asterix-gerrit.ics.uci.edu/#/c/1331/1/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualMetaDataPageManager.java
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualMetaDataPageManager.java:

Line 189:     public void setRootPage(int rootPage) throws HyracksDataException {
> MAJOR SonarQube violation:
Done


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: Yes

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Michael Blow (Code Review)" <do...@asterixdb.incubator.apache.org>.
Michael Blow has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 1:

(1 comment)

https://asterix-gerrit.ics.uci.edu/#/c/1331/1/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java:

Line 130:         buf.putInt(ROOT_PAGE_NUMBER,0);
> Oh, sonar doesn't check this?
Yeah, it does others of these, need to figure out why not this occurrence


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: Yes

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Ian Maxon (Code Review)" <do...@asterixdb.incubator.apache.org>.
Hello Jenkins,

I'd like you to reexamine a change.  Please visit

    https://asterix-gerrit.ics.uci.edu/1331

to look at the new patch set (#2).

Change subject: Fix for ASTERIXDB-1725
......................................................................

Fix for ASTERIXDB-1725

This is tricky, the best method I could think to fix this involved simply
putting the root page number in the metadata page rather than trying to
calculate it somehow going backwards from the end of the file.

I also fixed a bug discovered by Mike B where getLSNOffset was not
taking into consideration page header sizes.

Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
---
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IMetaDataPageManager.java
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexMetaDataFrame.java
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java
M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualMetaDataPageManager.java
M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java
M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
M hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java
M hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java
M hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java
11 files changed, 104 insertions(+), 21 deletions(-)


  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/31/1331/2
-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 2
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 1: Integration-Tests+1

Integration Tests Successful

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

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 1:

Integration Tests Started https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-integration-tests/1086/

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 3:

Integration Tests Started https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-integration-tests/1087/

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 3
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 2:

Build Started https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-notopic/3240/

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 2
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Ian Maxon (Code Review)" <do...@asterixdb.incubator.apache.org>.
Ian Maxon has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 2:

(3 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1331/2/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java:

Line 94:     public int getPageSizeWithHeader(){
> MAJOR SonarQube violation:
Done


https://asterix-gerrit.ics.uci.edu/#/c/1331/2/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java:

Line 273:     public int getPageSizeWithHeader(){
> MAJOR SonarQube violation:
Done


https://asterix-gerrit.ics.uci.edu/#/c/1331/2/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java
File hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java:

Line 102:     public int getPageSizeWithHeader(){
> MAJOR SonarQube violation:
Done


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 2
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: Yes

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 1: Integration-Tests-1

Integration Tests Timed Out

https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-integration-tests/1072/ : ABORTED

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Michael Blow (Code Review)" <do...@asterixdb.incubator.apache.org>.
Michael Blow has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 1:

(3 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1331/1/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java:

Line 130:         buf.putInt(ROOT_PAGE_NUMBER,0);
/,0/, 0/


https://asterix-gerrit.ics.uci.edu/#/c/1331/1/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java:

Line 473:             return ((long) metadataPageNum * (bufferCache.getPageSize() + BufferCache.RESERVED_HEADER_BYTES))
Should we push getPageSizeWithHeader() to IBufferCache and reference it here?


Line 501:     public void setRootPage(int rootPage) throws HyracksDataException{
> MAJOR SonarQube violation:
+1


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: Yes

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Ian Maxon (Code Review)" <do...@asterixdb.incubator.apache.org>.
Hello Jenkins,

I'd like you to reexamine a change.  Please visit

    https://asterix-gerrit.ics.uci.edu/1331

to look at the new patch set (#3).

Change subject: Fix for ASTERIXDB-1725
......................................................................

Fix for ASTERIXDB-1725

This is tricky, the best method I could think to fix this involved simply
putting the root page number in the metadata page rather than trying to
calculate it somehow going backwards from the end of the file.

I also fixed a bug discovered by Mike B where getLSNOffset was not
taking into consideration page header sizes.

Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
---
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IMetaDataPageManager.java
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexMetaDataFrame.java
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java
M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualMetaDataPageManager.java
M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java
M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
M hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java
M hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java
M hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java
11 files changed, 109 insertions(+), 26 deletions(-)


  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/31/1331/3
-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 3
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 1:

Build Started https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-notopic/3221/

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 1:

Integration Tests Started https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-integration-tests/1072/

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 1:

Integration Tests Started https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-integration-tests/1076/

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 1:

Integration Tests Started https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-integration-tests/1084/

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Ian Maxon (Code Review)" <do...@asterixdb.incubator.apache.org>.
Ian Maxon has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 1:

(2 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1331/1/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java:

Line 130:         buf.putInt(ROOT_PAGE_NUMBER,0);
> /,0/, 0/
Oh, sonar doesn't check this?


https://asterix-gerrit.ics.uci.edu/#/c/1331/1/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java:

Line 473:             return ((long) metadataPageNum * (bufferCache.getPageSize() + BufferCache.RESERVED_HEADER_BYTES))
> Should we push getPageSizeWithHeader() to IBufferCache and reference it her
That was what I wanted to do but I wasn't sure if there was a reason it was not exposed. However if the author of that method is puzzled about why I didn't do that, it may be a bad sign for my reasoning ;)


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: Yes

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 1:

Integration Tests Timed Out

https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-integration-tests/1084/ : ABORTED

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 3:

Build Started https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-notopic/3241/

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 3
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 1:

Integration Tests Timed Out

https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-integration-tests/1076/ : ABORTED

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: No

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Ian Maxon (Code Review)" <do...@asterixdb.incubator.apache.org>.
Ian Maxon has submitted this change and it was merged.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Fix for ASTERIXDB-1725

This is tricky, the best method I could think to fix this involved simply
putting the root page number in the metadata page rather than trying to
calculate it somehow going backwards from the end of the file.

I also fixed a bug discovered by Mike B where getLSNOffset was not
taking into consideration page header sizes.

Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Reviewed-on: https://asterix-gerrit.ics.uci.edu/1331
Sonar-Qube: Jenkins <je...@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
Reviewed-by: Michael Blow <mb...@apache.org>
---
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IMetaDataPageManager.java
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexMetaDataFrame.java
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java
M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java
M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualMetaDataPageManager.java
M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java
M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
M hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java
M hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java
M hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java
11 files changed, 109 insertions(+), 26 deletions(-)

Approvals:
  Michael Blow: Looks good to me, approved
  Jenkins: Verified; No violations found; Verified



diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IMetaDataPageManager.java b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IMetaDataPageManager.java
index 3982a3a..5d87f99 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IMetaDataPageManager.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IMetaDataPageManager.java
@@ -172,4 +172,8 @@
     long getLSNOffset() throws HyracksDataException;
 
     public long getLastMarkerLSN() throws HyracksDataException;
+
+    void setRootPage(int rootPage) throws HyracksDataException;
+
+    int getRootPage() throws HyracksDataException;
 }
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexMetaDataFrame.java b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexMetaDataFrame.java
index d4fbaa2..21e918d 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexMetaDataFrame.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexMetaDataFrame.java
@@ -25,7 +25,7 @@
 
     //Storage version #. Change this if you alter any tree frame formats to stop
     // possible corruption from old versions reading new formats.
-    public static final int VERSION = 2;
+    public static final int VERSION = 3;
 
     public void initBuffer(byte level);
 
@@ -74,4 +74,8 @@
     public long getLastMarkerLSN();
 
     public void setLastMarkerLSN(long lsn);
+
+    void setRootPageNumber(int rootPage);
+
+    int getRootPageNumber();
 }
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java
index d4194c2..5489dcc 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/LIFOMetaDataFrame.java
@@ -49,7 +49,8 @@
     public static final int LSN_OFFSET = ADDITIONAL_FILTERING_PAGE_OFFSET + 4; // 33
     private static final int LAST_MARKER_LSN_OFFSET = LSN_OFFSET + 8; // 41
     public static final int STORAGE_VERSION_OFFSET = LAST_MARKER_LSN_OFFSET + 4; //45
-    private static final int HEADER_END_OFFSET = LAST_MARKER_LSN_OFFSET + 4; //49
+    public static final int ROOT_PAGE_NUMBER = STORAGE_VERSION_OFFSET + 4; //49
+    private static final int HEADER_END_OFFSET = ROOT_PAGE_NUMBER + 4; // 53
 
     protected ICachedPage page = null;
     protected ByteBuffer buf = null;
@@ -126,6 +127,7 @@
         buf.putInt(NEXT_PAGE_OFFSET, -1);
         buf.putInt(ADDITIONAL_FILTERING_PAGE_OFFSET, -1);
         buf.putLong(LAST_MARKER_LSN_OFFSET, -1L);
+        buf.putInt(ROOT_PAGE_NUMBER, 0);
         buf.putInt(STORAGE_VERSION_OFFSET, VERSION);
         setValid(false);
     }
@@ -192,4 +194,14 @@
     public void setLSMComponentFilterPageId(int filterPage) {
         buf.putInt(ADDITIONAL_FILTERING_PAGE_OFFSET, filterPage);
     }
+
+    @Override
+    public void setRootPageNumber(int rootPage) {
+        buf.putInt(ROOT_PAGE_NUMBER, rootPage);
+    }
+
+    @Override
+    public int getRootPageNumber() {
+        return buf.getInt(ROOT_PAGE_NUMBER);
+    }
 }
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java
index e7a1123..2b0566d 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java
@@ -18,7 +18,6 @@
  */
 package org.apache.hyracks.storage.am.common.freepage;
 
-
 import org.apache.hyracks.api.exceptions.HyracksDataException;
 import org.apache.hyracks.storage.am.common.api.IMetaDataPageManager;
 import org.apache.hyracks.storage.am.common.api.ITreeIndexMetaDataFrame;
@@ -64,7 +63,7 @@
                 int newPage = metaFrame.getFreePage();
                 if (newPage < 0) {
                     throw new HyracksDataException(
-                              "Inconsistent Meta Page State. It has no space, but it also has no entries.");
+                            "Inconsistent Meta Page State. It has no space, but it also has no entries.");
                 }
 
                 ICachedPage newNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, newPage), false);
@@ -471,7 +470,7 @@
     public long getLSNOffset() throws HyracksDataException {
         int metadataPageNum = getFirstMetadataPage();
         if (metadataPageNum != IBufferCache.INVALID_PAGEID) {
-            return ((long)metadataPageNum * bufferCache.getPageSize()) + LIFOMetaDataFrame.LSN_OFFSET;
+            return ((long) metadataPageNum * bufferCache.getPageSizeWithHeader()) + LIFOMetaDataFrame.LSN_OFFSET;
         }
         return IMetaDataPageManager.INVALID_LSN_OFFSET;
     }
@@ -496,4 +495,48 @@
             }
         }
     }
+
+    @Override
+    public void setRootPage(int rootPage) throws HyracksDataException {
+        ICachedPage metaNode;
+        if (!appendOnly) {
+            metaNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, getFirstMetadataPage()), false);
+        } else {
+            metaNode = confiscatedMetaNode;
+        }
+        ITreeIndexMetaDataFrame metaFrame = metaDataFrameFactory.createFrame();
+        metaNode.acquireWriteLatch();
+        try {
+            metaFrame.setPage(metaNode);
+            metaFrame.setRootPageNumber(rootPage);
+        } finally {
+            if (!appendOnly) {
+                metaNode.releaseWriteLatch(true);
+                bufferCache.unpin(metaNode);
+            } else {
+                metaNode.releaseWriteLatch(false);
+            }
+        }
+    }
+
+    @Override
+    public int getRootPage() throws HyracksDataException {
+        ICachedPage metaNode;
+        if (!appendOnly || confiscatedMetaNode == null) {
+            metaNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, getFirstMetadataPage()), false);
+        } else {
+            metaNode = confiscatedMetaNode;
+        }
+        ITreeIndexMetaDataFrame metaFrame = metaDataFrameFactory.createFrame();
+        metaNode.acquireReadLatch();
+        try {
+            metaFrame.setPage(metaNode);
+            return metaFrame.getRootPageNumber();
+        } finally {
+            metaNode.releaseReadLatch();
+            if (!appendOnly || confiscatedMetaNode == null) {
+                bufferCache.unpin(metaNode);
+            }
+        }
+    }
 }
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java
index 0dcfc90..5ac203e 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java
@@ -114,7 +114,6 @@
         }
 
         freePageManager.open(fileId);
-        setRootAndMetadataPages(appendOnly);
         if (!appendOnly) {
             initEmptyTree();
             freePageManager.close();
@@ -122,6 +121,7 @@
             this.appendOnly = true;
             initCachedMetadataPage();
         }
+        setRootPage(appendOnly);
         bufferCache.closeFile(fileId);
     }
 
@@ -140,25 +140,14 @@
         }
     }
 
-    private void setRootAndMetadataPages(boolean appendOnly) throws HyracksDataException {
+    private void setRootPage(boolean appendOnly) throws HyracksDataException {
         if (!appendOnly) {
             // regular or empty tree
             rootPage = 1;
             bulkloadLeafStart = 2;
         } else {
-            //the root page is either page n-2 (no filter) or n-3 (filter)
-            int numPages = bufferCache.getNumPagesOfFile(fileId);
-            if (numPages > MINIMAL_TREE_PAGE_COUNT) {
-                int filterPageId = freePageManager.getFilterPageId();
-                if (filterPageId > 0) {
-                    rootPage = numPages - MINIMAL_TREE_PAGE_COUNT_WITH_FILTER;
-                } else {
-                    rootPage = numPages - MINIMAL_TREE_PAGE_COUNT;
-                }
-            } else {
-                rootPage = 0;
-            }
-
+            //root page is stored in MD page
+            rootPage = freePageManager.getRootPage();
             //leaves start from the very beginning of the file.
             bulkloadLeafStart = 0;
         }
@@ -201,7 +190,7 @@
         } else {
             appendOnly = false;
         }
-        setRootAndMetadataPages(appendOnly);
+        setRootPage(appendOnly);
 
         // TODO: Should probably have some way to check that the tree is physically consistent
         // or that the file we just opened actually is a tree
@@ -424,6 +413,7 @@
 
                 }
             }
+            freePageManager.setRootPage(rootPage);
         }
 
         protected void addLevel() throws HyracksDataException {
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualMetaDataPageManager.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualMetaDataPageManager.java
index 158c68f..eab85d2 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualMetaDataPageManager.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualMetaDataPageManager.java
@@ -184,4 +184,17 @@
         // Method doesn't make sense for this free page manager.
         return -1L;
     }
+
+    @Override
+    public void setRootPage(int rootPage) throws HyracksDataException {
+        // This won't get called for an in-place index. The root page
+        // is maintained at a fixed location as in the below method.
+    }
+
+    @Override
+    public int getRootPage() throws HyracksDataException {
+        // This also won't be called but the correct answer for an
+        // In-place index is always 1.
+        return 1;
+    }
 }
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java
index 748dbfe..3ef419e 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java
@@ -91,6 +91,10 @@
         return vbc.getPageSize();
     }
 
+    public int getPageSizeWithHeader() {
+        return vbc.getPageSizeWithHeader();
+    }
+
     @Override
     public int getNumPages() {
         return vbc.getNumPages();
@@ -103,7 +107,6 @@
             vbc.close();
         }
     }
-
 
     @Override
     public synchronized void open() throws HyracksDataException {
@@ -194,7 +197,7 @@
     }
 
     @Override
-    public int getFileReferenceCount(int fileId){
+    public int getFileReferenceCount(int fileId) {
         return 0;
     }
 
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
index c8ce00f..abfc35c 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
@@ -248,8 +248,8 @@
         } else {
             largePages.getAndAdd(multiplier - origMultiplier);
         }
-        ((VirtualPage)cPage).buffer = newBuffer;
-        ((VirtualPage)cPage).multiplier = multiplier;
+        ((VirtualPage) cPage).buffer = newBuffer;
+        ((VirtualPage) cPage).multiplier = multiplier;
     }
 
     @Override
@@ -266,6 +266,11 @@
 
     @Override
     public int getPageSize() {
+        return pageSize;
+    }
+
+    @Override
+    public int getPageSizeWithHeader() {
         return pageSize;
     }
 
@@ -335,7 +340,7 @@
 
     @Override
     public boolean isFull() {
-        return (nextFree  + largePages.get()) >= numPages;
+        return (nextFree + largePages.get()) >= numPages;
     }
 
     private static class CacheBucket {
@@ -390,6 +395,7 @@
         public void releaseWriteLatch(boolean markDirty) {
             latch.writeLock().unlock();
         }
+
         public boolean confiscated() {
             return false;
         }
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java
index 3be1c46..ed33cc4 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java
@@ -132,6 +132,7 @@
         return pageSize;
     }
 
+    @Override
     public int getPageSizeWithHeader() {
         return pageSize + RESERVED_HEADER_BYTES;
     }
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java
index 4a41ec0..6c88275 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java
@@ -99,6 +99,11 @@
     }
 
     @Override
+    public int getPageSizeWithHeader() {
+        return bufferCache.getPageSizeWithHeader();
+    }
+
+    @Override
     public int getNumPages() {
         return bufferCache.getNumPages();
     }
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java
index ca87673..27e7982 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java
@@ -62,6 +62,8 @@
 
     int getPageSize();
 
+    int getPageSizeWithHeader();
+
     public int getNumPages();
 
     public int getNumPagesOfFile(int fileId) throws HyracksDataException;

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 4
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>

Change in asterixdb[master]: Fix for ASTERIXDB-1725

Posted by "Michael Blow (Code Review)" <do...@asterixdb.incubator.apache.org>.
Michael Blow has posted comments on this change.

Change subject: Fix for ASTERIXDB-1725
......................................................................


Patch Set 3: Code-Review+2

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1331
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f21e96ab045d331e4fc1c77b5c73b975e6260fa
Gerrit-PatchSet: 3
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: No