You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@asterixdb.apache.org by "Murtadha Hubail (Code Review)" <do...@asterix-gerrit.ics.uci.edu> on 2015/08/07 17:02:43 UTC
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Murtadha Hubail has uploaded a new change for review.
https://asterix-gerrit.ics.uci.edu/343
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Add support for persistent local resources to IndexLifecycleManager
Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
---
M hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IIndexLifecycleManager.java
M hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java
M hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
M hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java
M hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/impls/NoOpOperationCallbackFactory.java
M hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
M hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java
M hyracks/hyracks-storage-am-lsm-rtree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java
8 files changed, 158 insertions(+), 40 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/hyracks refs/changes/43/343/1
diff --git a/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IIndexLifecycleManager.java b/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IIndexLifecycleManager.java
index de4ce5a..2f3fdb2 100644
--- a/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IIndexLifecycleManager.java
+++ b/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IIndexLifecycleManager.java
@@ -30,5 +30,18 @@
public void close(long resourceID) throws HyracksDataException;
public List<IIndex> getOpenIndexes();
-
+
+ public boolean supportsPersistentLocalResources();
+
+ public void register(String resourceName, IIndex index) throws HyracksDataException;
+
+ public void open(String resourceName) throws HyracksDataException;
+
+ public void close(String resourceName) throws HyracksDataException;
+
+ public IIndex getIndex(String resourceName) throws HyracksDataException;
+
+ public void unregister(String resourceName) throws HyracksDataException;
+
+ public IIndex getIndex(int datasetID, long resourceID) throws HyracksDataException;
}
\ No newline at end of file
diff --git a/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java b/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java
index 968acd0..1d85256 100644
--- a/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java
+++ b/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java
@@ -20,6 +20,9 @@
import edu.uci.ics.hyracks.api.exceptions.HyracksDataException;
public interface IModificationOperationCallbackFactory extends Serializable {
- public IModificationOperationCallback createModificationOperationCallback(long resourceId, Object resource, IHyracksTaskContext ctx)
- throws HyracksDataException;
+ public IModificationOperationCallback createModificationOperationCallback(long resourceId, Object resource,
+ IHyracksTaskContext ctx) throws HyracksDataException;
+
+ public IModificationOperationCallback createModificationOperationCallback(String resourceName, long resourceId,
+ Object resource, IHyracksTaskContext ctx) throws HyracksDataException;
}
diff --git a/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java b/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
index d67f585..816d743 100644
--- a/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
+++ b/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
@@ -69,11 +69,21 @@
public void create() throws HyracksDataException {
synchronized (lcManager) {
long resourceID = getResourceID();
- index = lcManager.getIndex(resourceID);
- if (index != null) {
- lcManager.unregister(resourceID);
+
+ if (lcManager.supportsPersistentLocalResources()) {
+ index = lcManager.getIndex(file.getFile().getPath());
+ if (index != null) {
+ lcManager.unregister(file.getFile().getPath());
+ } else {
+ index = createIndexInstance();
+ }
} else {
- index = createIndexInstance();
+ index = lcManager.getIndex(resourceID);
+ if (index != null) {
+ lcManager.unregister(resourceID);
+ } else {
+ index = createIndexInstance();
+ }
}
// The previous resource ID needs to be removed since calling IIndex.create() may possibly destroy
@@ -93,7 +103,12 @@
} catch (IOException e) {
throw new HyracksDataException(e);
}
- lcManager.register(resourceID, index);
+
+ if (lcManager.supportsPersistentLocalResources()) {
+ lcManager.register(file.getFile().getPath(), index);
+ } else {
+ lcManager.register(resourceID, index);
+ }
}
}
@@ -106,19 +121,32 @@
throw new HyracksDataException("Index does not have a valid resource ID. Has it been created yet?");
}
- index = lcManager.getIndex(resourceID);
- if (index == null) {
- index = createIndexInstance();
- lcManager.register(resourceID, index);
+ if (lcManager.supportsPersistentLocalResources()) {
+ index = lcManager.getIndex(file.getFile().getPath());
+ if (index == null) {
+ index = createIndexInstance();
+ lcManager.register(file.getFile().getPath(), index);
+ }
+ lcManager.open(file.getFile().getPath());
+ } else {
+ index = lcManager.getIndex(resourceID);
+ if (index == null) {
+ index = createIndexInstance();
+ lcManager.register(resourceID, index);
+ }
+ lcManager.open(resourceID);
}
- lcManager.open(resourceID);
}
}
@Override
public void close() throws HyracksDataException {
synchronized (lcManager) {
- lcManager.close(getResourceID());
+ if (lcManager.supportsPersistentLocalResources()) {
+ lcManager.close(file.getFile().getPath());
+ } else {
+ lcManager.close(getResourceID());
+ }
}
}
@@ -126,11 +154,20 @@
public void destroy() throws HyracksDataException {
synchronized (lcManager) {
long resourceID = getResourceID();
- index = lcManager.getIndex(resourceID);
- if (index != null) {
- lcManager.unregister(resourceID);
+ if (lcManager.supportsPersistentLocalResources()) {
+ index = lcManager.getIndex(file.getFile().getPath());
+ if (index != null) {
+ lcManager.unregister(file.getFile().getPath());
+ } else {
+ index = createIndexInstance();
+ }
} else {
- index = createIndexInstance();
+ index = lcManager.getIndex(resourceID);
+ if (index != null) {
+ lcManager.unregister(resourceID);
+ } else {
+ index = createIndexInstance();
+ }
}
if (resourceID != -1) {
diff --git a/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java b/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java
index 8543433..970ef93 100644
--- a/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java
+++ b/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java
@@ -231,4 +231,39 @@
}
os.write(sb.toString().getBytes());
}
+
+ @Override
+ public void register(String resourceName, IIndex index) throws HyracksDataException {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void open(String resourceName) throws HyracksDataException {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public boolean supportsPersistentLocalResources() {
+ return false;
+ }
+
+ @Override
+ public void close(String resourceName) throws HyracksDataException {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public IIndex getIndex(String resourceName) throws HyracksDataException {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void unregister(String resourceName) throws HyracksDataException {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public IIndex getIndex(int datasetID, long resourceID) throws HyracksDataException {
+ throw new UnsupportedOperationException();
+ }
}
\ No newline at end of file
diff --git a/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/impls/NoOpOperationCallbackFactory.java b/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/impls/NoOpOperationCallbackFactory.java
index 7a9ae4c..a0f678c 100644
--- a/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/impls/NoOpOperationCallbackFactory.java
+++ b/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/impls/NoOpOperationCallbackFactory.java
@@ -15,6 +15,7 @@
package edu.uci.ics.hyracks.storage.am.common.impls;
import edu.uci.ics.hyracks.api.context.IHyracksTaskContext;
+import edu.uci.ics.hyracks.api.exceptions.HyracksDataException;
import edu.uci.ics.hyracks.storage.am.common.api.IModificationOperationCallback;
import edu.uci.ics.hyracks.storage.am.common.api.IModificationOperationCallbackFactory;
import edu.uci.ics.hyracks.storage.am.common.api.ISearchOperationCallback;
@@ -29,7 +30,8 @@
INSTANCE;
@Override
- public IModificationOperationCallback createModificationOperationCallback(long resourceId, Object resource, IHyracksTaskContext ctx) {
+ public IModificationOperationCallback createModificationOperationCallback(long resourceId, Object resource,
+ IHyracksTaskContext ctx) {
return NoOpOperationCallback.INSTANCE;
}
@@ -37,4 +39,10 @@
public ISearchOperationCallback createSearchOperationCallback(long resourceId, IHyracksTaskContext ctx) {
return NoOpOperationCallback.INSTANCE;
}
+
+ @Override
+ public IModificationOperationCallback createModificationOperationCallback(String resourceName, long resourceId,
+ Object resource, IHyracksTaskContext ctx) throws HyracksDataException {
+ return NoOpOperationCallback.INSTANCE;
+ }
}
diff --git a/hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java b/hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
index b913e28..87a1acf 100644
--- a/hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
+++ b/hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
@@ -36,8 +36,7 @@
public ExternalBTreeDataflowHelper(IIndexOperatorDescriptor opDesc, IHyracksTaskContext ctx, int partition,
double bloomFilterFalsePositiveRate, ILSMMergePolicy mergePolicy,
ILSMOperationTrackerProvider opTrackerFactory, ILSMIOOperationScheduler ioScheduler,
- ILSMIOOperationCallbackFactory ioOpCallbackFactory, boolean needKeyDupCheck, int version,
- boolean durable) {
+ ILSMIOOperationCallbackFactory ioOpCallbackFactory, boolean needKeyDupCheck, int version, boolean durable) {
super(opDesc, ctx, partition, null, bloomFilterFalsePositiveRate, mergePolicy, opTrackerFactory, ioScheduler,
ioOpCallbackFactory, false, null, null, null, null, durable);
this.version = version;
@@ -46,8 +45,7 @@
public ExternalBTreeDataflowHelper(IIndexOperatorDescriptor opDesc, IHyracksTaskContext ctx, int partition,
List<IVirtualBufferCache> virtualBufferCaches, ILSMMergePolicy mergePolicy,
ILSMOperationTrackerProvider opTrackerFactory, ILSMIOOperationScheduler ioScheduler,
- ILSMIOOperationCallbackFactory ioOpCallbackFactory, boolean needKeyDupCheck, int version,
- boolean durable) {
+ ILSMIOOperationCallbackFactory ioOpCallbackFactory, boolean needKeyDupCheck, int version, boolean durable) {
this(opDesc, ctx, partition, DEFAULT_BLOOM_FILTER_FALSE_POSITIVE_RATE, mergePolicy, opTrackerFactory,
ioScheduler, ioOpCallbackFactory, needKeyDupCheck, version, durable);
}
@@ -63,10 +61,18 @@
} catch (HyracksDataException e) {
return null;
}
- try {
- index = lcManager.getIndex(resourceID);
- } catch (HyracksDataException e) {
- return null;
+ if (lcManager.supportsPersistentLocalResources()) {
+ try {
+ index = lcManager.getIndex(file.getFile().getPath());
+ } catch (HyracksDataException e) {
+ return null;
+ }
+ } else {
+ try {
+ index = lcManager.getIndex(resourceID);
+ } catch (HyracksDataException e) {
+ return null;
+ }
}
}
return index;
diff --git a/hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java b/hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java
index ff19c54..075b3ed 100644
--- a/hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java
+++ b/hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java
@@ -63,10 +63,18 @@
} catch (HyracksDataException e) {
return null;
}
- try {
- index = lcManager.getIndex(resourceID);
- } catch (HyracksDataException e) {
- return null;
+ if (lcManager.supportsPersistentLocalResources()) {
+ try {
+ index = lcManager.getIndex(file.getFile().getPath());
+ } catch (HyracksDataException e) {
+ return null;
+ }
+ } else {
+ try {
+ index = lcManager.getIndex(resourceID);
+ } catch (HyracksDataException e) {
+ return null;
+ }
}
}
return index;
diff --git a/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java b/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java
index 4fee495..252775f 100644
--- a/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java
+++ b/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java
@@ -77,10 +77,18 @@
} catch (HyracksDataException e) {
return null;
}
- try {
- index = lcManager.getIndex(resourceID);
- } catch (HyracksDataException e) {
- return null;
+ if (lcManager.supportsPersistentLocalResources()) {
+ try {
+ index = lcManager.getIndex(file.getFile().getPath());
+ } catch (HyracksDataException e) {
+ return null;
+ }
+ } else {
+ try {
+ index = lcManager.getIndex(resourceID);
+ } catch (HyracksDataException e) {
+ return null;
+ }
}
}
return index;
@@ -95,11 +103,11 @@
ITypeTraits[] filterTypeTraits, IBinaryComparatorFactory[] filterCmpFactories, int[] filterFields)
throws HyracksDataException {
try {
- return LSMRTreeUtils.createExternalRTree(file, diskBufferCache, diskFileMapProvider, typeTraits,
- rtreeCmpFactories, btreeCmpFactories, valueProviderFactories, rtreePolicyType,
- bloomFilterFalsePositiveRate, mergePolicy, opTracker, ioScheduler,
- ioOpCallbackFactory.createIOOperationCallback(), linearizeCmpFactory, btreeFields, version,
- durable);
+ return LSMRTreeUtils
+ .createExternalRTree(file, diskBufferCache, diskFileMapProvider, typeTraits, rtreeCmpFactories,
+ btreeCmpFactories, valueProviderFactories, rtreePolicyType, bloomFilterFalsePositiveRate,
+ mergePolicy, opTracker, ioScheduler, ioOpCallbackFactory.createIOOperationCallback(),
+ linearizeCmpFactory, btreeFields, version, durable);
} catch (TreeIndexException e) {
throw new HyracksDataException(e);
}
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 1
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Murtadha Hubail (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Hello Jenkins,
I'd like you to reexamine a change. Please visit
https://asterix-gerrit.ics.uci.edu/343
to look at the new patch set (#2).
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Add support for persistent local resources to IndexLifecycleManager
Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
---
M hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IIndexLifecycleManager.java
M hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java
M hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
M hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java
M hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/impls/NoOpOperationCallbackFactory.java
M hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
M hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java
M hyracks/hyracks-storage-am-lsm-rtree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java
8 files changed, 158 insertions(+), 40 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/hyracks refs/changes/43/343/2
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Murtadha Hubail (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Murtadha Hubail has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 2:
Yes, the goal is correct, and you dug correctly. In the logs, we have the dataset id and the resource id. We don't store the string.
During recovery (REDO only), we load all resources and construct the mapping while loading them.
During aborts (UNDO), the indexes are guaranteed to be open, so we fetch them using the dataset id and the resource id. This mapping is maintained in DatasetLifeCycleManager for the opened datasets only, and removed once they are closed.
Therefore, strings are enough for the API between IndexDataFlowHelper and IndexLifeCycleManager.
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Murtadha Hubail (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Murtadha Hubail has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 3:
(5 comments)
https://asterix-gerrit.ics.uci.edu/#/c/343/3/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
File hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java:
Line 59: index = lcManager.getIndex(file.getFile().getPath());
> create a local variable for "file.getFile().getPath()"?
Used same variable from super class.
https://asterix-gerrit.ics.uci.edu/#/c/343/3/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java
File hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java:
Line 60: index = lcManager.getIndex(file.getFile().getPath());
> create a local variable for "file.getFile().getPath()"?
Used same variable from super class.
https://asterix-gerrit.ics.uci.edu/#/c/343/3/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java
File hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java:
Line 75: index = lcManager.getIndex(file.getFile().getPath());
> create a local variable for "file.getFile().getPath()"?
Used same variable from super class.
https://asterix-gerrit.ics.uci.edu/#/c/343/3/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ILocalResourceRepository.java
File hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ILocalResourceRepository.java:
Line 21: public LocalResource getResourceById(long id) throws HyracksDataException;
> Do we still need this? Because your Asterix CL throws unsupported exceptio
Removed.
Line 28: public void deleteResourceById(long id) throws HyracksDataException;
> Do we still need this? Because your Asterix CL throws unsupported exceptio
Removed.
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 3
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: Yes
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Murtadha Hubail (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Murtadha Hubail has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 2:
(1 comment)
https://asterix-gerrit.ics.uci.edu/#/c/343/2/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IIndexLifecycleManager.java
File hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IIndexLifecycleManager.java:
Line 37:
> Well, if the string presents in the map (which is the most-often case), ac
It's a HashMap. Actual string comparison is only done when there are collisions. Besides, as you mentioned, the frequency of these lookups is very low compared to other operations. I doubt that this is going to be our bottleneck.
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: Yes
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Yingyi Bu (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Yingyi Bu has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 2:
(2 comments)
https://asterix-gerrit.ics.uci.edu/#/c/343/2/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IIndexLifecycleManager.java
File hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IIndexLifecycleManager.java:
Line 37:
> I would like to keep the string only as well, but I don't want to remove th
Well, if the string presents in the map (which is the most-often case), actual string comparisons will be called. But I think that anyway shouldn't be the bottleneck of our current system --- because we haven't run thousands of lookups/updates simultaneously yet.
https://asterix-gerrit.ics.uci.edu/#/c/343/2/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java
File hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java:
Line 27: Object resource, IHyracksTaskContext ctx) throws HyracksDataException;
> Yes, I don't think it is used.
OK, i'm fine to keep that in the current CL and maybe we can remove that in a future CL.
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: Yes
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Jenkins (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Jenkins has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 2: Verified+1
Build Successful
https://asterix-jenkins.ics.uci.edu/job/hyracks-gerrit/412/ : SUCCESS
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Murtadha Hubail (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Murtadha Hubail has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 2:
To answer your first point, here is some background behind the reason of this change.
a) A resource location (Name) is passed to IndexDataFlowHelper as an absolute path from a FileSplit.
b) The current design of PersistentLocalResourceRepository assumes that the mapping information between (Resource Name -> LocalResource) is already in memory, and all resources info is loaded in memory during startup.
c) We also need to keep a different mapping information between (ID -> LocalResource) since the API of IIndexLifecycleManager (DatasetLifecycleManager in Asterix) is only using the resource id, so if it needs to access any other info regarding this resource (e.g. Dataset ID), it needs to use the (ID -> LocalResource) map. If we have a huge number of resources, all of that info will be kept in memory all the time until a resource is destroyed.
I fixed this on the Asterix side of this change and made PersistentLocalResourceRepository load resources on demand and there is a cache between (Resource Name -> LocalResource) with a certain max size and got rid of the two mappings. But now DatasetLifecycleManager won’t be able to access the resource using the ID only. That’s why I introduced this change here to pass the name instead of the id. Using the Name, it will ask PersistentLocalResourceRepository for the resource using its Name and it will be fetched from the cache or disk.
Another option I could’ve done is to pass the whole LocalResource instead of the name. This way DatasetLifecycleManager wouldn’t need to fetch it again at all. But then I found that we have a logic that is based on checking whether that resource actually exists or not.
Coming back to the rest of your points:
2) I could change the interface to use Object, but it will again break the existing code of other clients.
3) If we all agree that String is the better identifier, I’m going to add deprecated to the resource ID methods.
4) Clients do not need to make this check since they should know what type of ILocalResourceRepository they are using. I added this check here because IndexDataflowHelper is a shared class between our clients. New clients could create a new class which implements IIndexDataflowHelper that doesn’t need to make this check. Hopefully we could implement point 3 and git rid of this check as well. I also wanted to make this check based on instanceof instead of a boolean but couldn’t due to projects dependancies.
Hope this answers your points.
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Murtadha Hubail (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Hello abdullah alamoudi, Yingyi Bu, Till Westmann, Jenkins,
I'd like you to reexamine a change. Please visit
https://asterix-gerrit.ics.uci.edu/343
to look at the new patch set (#4).
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Add support for persistent local resources to IndexLifecycleManager
Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
---
M hyracks/hyracks-examples/btree-example/btreehelper/src/main/java/org/apache/hyracks/examples/btree/helper/RuntimeContext.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexDataflowHelper.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexLifecycleManager.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/NoOpOperationCallbackFactory.java
M hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
M hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java
M hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ILocalResourceRepository.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactory.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactoryProvider.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/TransientLocalResourceRepository.java
M hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/test/support/TestStorageManagerComponentHolder.java
15 files changed, 157 insertions(+), 182 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/hyracks refs/changes/43/343/4
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 4
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Jenkins (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Jenkins has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 1:
Build Started https://asterix-jenkins.ics.uci.edu/job/hyracks-gerrit/400/
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 1
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Till Westmann (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Till Westmann has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 2:
Just a question:
This change introduces a way to identify resources by name in addition to identifying them by id which increases the size of the IIndexLifecycleManager interface and introduces some code duplication in a few files. Were other alternatives considered?
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Jenkins (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Jenkins has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 1: Verified+1
Build Successful
https://asterix-jenkins.ics.uci.edu/job/hyracks-gerrit/400/ : SUCCESS
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 1
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Till Westmann (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Till Westmann has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 2:
I have a few thoughts, but not necessarily a concrete suggestion (as I don't really understand the requirements here).
1) It seems that longs are a nice and efficient way to identify resources. And we generally seem to use maps to map between these longs and the actual resources. But that doesn't seem to be feasible here. It would be nice to understand why.
2) If the resource identifier would be changed to be an Object, it could either be a (boxed) Long or a String, so the client source code using longs would not have to change. However other implementations of the interface should be changed. Also this might just keep the interface small and create some ugly type-based handling behind the interface.
3) If it actually turns out that Strings are the better identifiers here for everything, we should probably deprecate the other methods such that the other clients migrate away from those interfaces and we can tighten the interface again.
4) I'm concerned about additional maintenance burden introduced by the client code that takes decisions based on "lcManager.supportsPersistentLocalResources()". It would be nice if such decisions (if necessary) would be taken in one reusable piece of code and not by every client of the interface.
Does this make sense?
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: ASTERIXDB-1053: change IndexLifecycleManager API to use reso...
Posted by "Ildar Absalyamov (Code Review)" <do...@asterixdb.incubator.apache.org>.
Hello abdullah alamoudi, Yingyi Bu, Till Westmann, Jenkins,
I'd like you to reexamine a change. Please visit
https://asterix-gerrit.ics.uci.edu/343
to look at the new patch set (#6).
Change subject: ASTERIXDB-1053: change IndexLifecycleManager API to use resource name
......................................................................
ASTERIXDB-1053: change IndexLifecycleManager API to use resource name
Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
---
M hyracks/hyracks-examples/btree-example/btreehelper/src/main/java/org/apache/hyracks/examples/btree/helper/RuntimeContext.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexDataflowHelper.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexLifecycleManager.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexInsertUpdateDeleteOperatorNodePushable.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/NoOpOperationCallbackFactory.java
M hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
M hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java
M hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ILocalResourceRepository.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactory.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactoryProvider.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/TransientLocalResourceRepository.java
M hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/test/support/TestStorageManagerComponentHolder.java
16 files changed, 150 insertions(+), 184 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/hyracks refs/changes/43/343/6
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 6
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Jenkins (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Jenkins has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 3:
Build Started https://asterix-jenkins.ics.uci.edu/job/hyracks-gerrit/431/
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 3
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: ASTERIXDB-1053: change IndexLifecycleManager API to use reso...
Posted by "Murtadha Hubail (Code Review)" <do...@asterixdb.incubator.apache.org>.
Murtadha Hubail has submitted this change and it was merged.
Change subject: ASTERIXDB-1053: change IndexLifecycleManager API to use resource name
......................................................................
ASTERIXDB-1053: change IndexLifecycleManager API to use resource name
Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Reviewed-on: https://asterix-gerrit.ics.uci.edu/343
Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
Reviewed-by: Yingyi Bu <bu...@gmail.com>
---
M hyracks/hyracks-examples/btree-example/btreehelper/src/main/java/org/apache/hyracks/examples/btree/helper/RuntimeContext.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexDataflowHelper.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexLifecycleManager.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexInsertUpdateDeleteOperatorNodePushable.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/NoOpOperationCallbackFactory.java
M hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
M hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java
M hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ILocalResourceRepository.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactory.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactoryProvider.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/TransientLocalResourceRepository.java
M hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/test/support/TestStorageManagerComponentHolder.java
16 files changed, 150 insertions(+), 184 deletions(-)
Approvals:
Yingyi Bu: Looks good to me, approved
Jenkins: Verified
diff --git a/hyracks/hyracks-examples/btree-example/btreehelper/src/main/java/org/apache/hyracks/examples/btree/helper/RuntimeContext.java b/hyracks/hyracks-examples/btree-example/btreehelper/src/main/java/org/apache/hyracks/examples/btree/helper/RuntimeContext.java
index 0f96c19..6fde446 100644
--- a/hyracks/hyracks-examples/btree-example/btreehelper/src/main/java/org/apache/hyracks/examples/btree/helper/RuntimeContext.java
+++ b/hyracks/hyracks-examples/btree-example/btreehelper/src/main/java/org/apache/hyracks/examples/btree/helper/RuntimeContext.java
@@ -60,10 +60,10 @@
IPageReplacementStrategy prs = new ClockPageReplacementStrategy(allocator, 32768, 50);
bufferCache = new BufferCache(appCtx.getRootContext().getIOManager(), prs, new DelayPageCleanerPolicy(1000),
fileMapManager, 100, threadFactory);
- lcManager = new IndexLifecycleManager();
ILocalResourceRepositoryFactory localResourceRepositoryFactory = new TransientLocalResourceRepositoryFactory();
localResourceRepository = localResourceRepositoryFactory.createRepository();
resourceIdFactory = (new ResourceIdFactoryProvider(localResourceRepository)).createResourceIdFactory();
+ lcManager = new IndexLifecycleManager(localResourceRepository);
}
public void close() throws HyracksDataException {
diff --git a/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexDataflowHelper.java b/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexDataflowHelper.java
index 0906efe..81d7834 100644
--- a/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexDataflowHelper.java
+++ b/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexDataflowHelper.java
@@ -36,6 +36,8 @@
public FileReference getFileReference();
public long getResourceID() throws HyracksDataException;
-
+
public IHyracksTaskContext getTaskContext();
+
+ public String getResourceName();
}
diff --git a/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexLifecycleManager.java b/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexLifecycleManager.java
index ad0fcef..41eb5d5 100644
--- a/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexLifecycleManager.java
+++ b/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexLifecycleManager.java
@@ -23,16 +23,17 @@
import org.apache.hyracks.api.exceptions.HyracksDataException;
public interface IIndexLifecycleManager {
- public IIndex getIndex(long resourceID) throws HyracksDataException;
-
- public void register(long resourceID, IIndex index) throws HyracksDataException;
-
- public void unregister(long resourceID) throws HyracksDataException;
-
- public void open(long resourceID) throws HyracksDataException;
-
- public void close(long resourceID) throws HyracksDataException;
-
public List<IIndex> getOpenIndexes();
-
+
+ public void register(String resourceName, IIndex index) throws HyracksDataException;
+
+ public void open(String resourceName) throws HyracksDataException;
+
+ public void close(String resourceName) throws HyracksDataException;
+
+ public IIndex getIndex(String resourceName) throws HyracksDataException;
+
+ public void unregister(String resourceName) throws HyracksDataException;
+
+ public IIndex getIndex(int datasetID, long resourceID) throws HyracksDataException;
}
\ No newline at end of file
diff --git a/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java b/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java
index 9d8093f..212190c 100644
--- a/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java
+++ b/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java
@@ -24,6 +24,6 @@
import org.apache.hyracks.api.exceptions.HyracksDataException;
public interface IModificationOperationCallbackFactory extends Serializable {
- public IModificationOperationCallback createModificationOperationCallback(long resourceId, Object resource, IHyracksTaskContext ctx)
- throws HyracksDataException;
+ public IModificationOperationCallback createModificationOperationCallback(String resourceName, long resourceId,
+ Object resource, IHyracksTaskContext ctx) throws HyracksDataException;
}
diff --git a/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java b/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
index 932994d..2b6b208 100644
--- a/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
+++ b/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
@@ -45,7 +45,7 @@
protected final int partition;
protected final int ioDeviceId;
protected final boolean durable;
-
+ protected final String resourceName;
protected IIndex index;
public IndexDataflowHelper(IIndexOperatorDescriptor opDesc, final IHyracksTaskContext ctx, int partition,
@@ -60,6 +60,7 @@
this.file = new FileReference(new File(IndexFileNameUtil.prepareFileName(opDesc.getFileSplitProvider()
.getFileSplits()[partition].getLocalFile().getFile().getPath(), ioDeviceId)));
this.durable = durable;
+ this.resourceName = file.getFile().getPath();
}
protected abstract IIndex createIndexInstance() throws HyracksDataException;
@@ -72,10 +73,9 @@
@Override
public void create() throws HyracksDataException {
synchronized (lcManager) {
- long resourceID = getResourceID();
- index = lcManager.getIndex(resourceID);
+ index = lcManager.getIndex(resourceName);
if (index != null) {
- lcManager.unregister(resourceID);
+ lcManager.unregister(resourceName);
} else {
index = createIndexInstance();
}
@@ -83,62 +83,59 @@
// The previous resource ID needs to be removed since calling IIndex.create() may possibly destroy
// any physical artifact that the LocalResourceRepository is managing (e.g. a file containing the resource ID).
// Once the index has been created, a new resource ID can be generated.
+ long resourceID = getResourceID();
if (resourceID != -1) {
- localResourceRepository.deleteResourceByName(file.getFile().getPath());
+ localResourceRepository.deleteResourceByName(resourceName);
}
index.create();
try {
- //TODO Create LocalResource through LocalResourceFactory interface
resourceID = resourceIdFactory.createId();
ILocalResourceFactory localResourceFactory = opDesc.getLocalResourceFactoryProvider()
.getLocalResourceFactory();
- localResourceRepository.insert(localResourceFactory.createLocalResource(resourceID, file.getFile()
- .getPath(), partition));
+ localResourceRepository.insert(localResourceFactory.createLocalResource(resourceID, resourceName,
+ partition));
} catch (IOException e) {
throw new HyracksDataException(e);
}
- lcManager.register(resourceID, index);
+ lcManager.register(resourceName, index);
}
}
@Override
public void open() throws HyracksDataException {
synchronized (lcManager) {
- long resourceID = getResourceID();
-
- if (resourceID == -1) {
+ if (getResourceID() == -1) {
throw new HyracksDataException("Index does not have a valid resource ID. Has it been created yet?");
}
- index = lcManager.getIndex(resourceID);
+ index = lcManager.getIndex(resourceName);
if (index == null) {
index = createIndexInstance();
- lcManager.register(resourceID, index);
+ lcManager.register(resourceName, index);
}
- lcManager.open(resourceID);
+ lcManager.open(resourceName);
}
}
@Override
public void close() throws HyracksDataException {
synchronized (lcManager) {
- lcManager.close(getResourceID());
+ lcManager.close(resourceName);
}
}
@Override
public void destroy() throws HyracksDataException {
synchronized (lcManager) {
- long resourceID = getResourceID();
- index = lcManager.getIndex(resourceID);
+ index = lcManager.getIndex(resourceName);
if (index != null) {
- lcManager.unregister(resourceID);
+ lcManager.unregister(resourceName);
} else {
index = createIndexInstance();
}
- if (resourceID != -1) {
- localResourceRepository.deleteResourceByName(file.getFile().getPath());
+ if (getResourceID() != -1) {
+ localResourceRepository.deleteResourceByName(resourceName);
}
index.destroy();
}
@@ -151,16 +148,17 @@
@Override
public long getResourceID() throws HyracksDataException {
- LocalResource localResource = localResourceRepository.getResourceByName(file.getFile().getPath());
- if (localResource == null) {
- return -1;
- } else {
- return localResource.getResourceId();
- }
+ LocalResource lr = localResourceRepository.getResourceByName(resourceName);
+ return lr == null ? -1 : lr.getResourceId();
}
@Override
public IHyracksTaskContext getTaskContext() {
return ctx;
}
+
+ @Override
+ public String getResourceName() {
+ return resourceName;
+ }
}
\ No newline at end of file
diff --git a/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexInsertUpdateDeleteOperatorNodePushable.java b/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexInsertUpdateDeleteOperatorNodePushable.java
index da32f87..4434871 100644
--- a/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexInsertUpdateDeleteOperatorNodePushable.java
+++ b/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexInsertUpdateDeleteOperatorNodePushable.java
@@ -75,7 +75,7 @@
indexHelper.open();
IIndex index = indexHelper.getIndexInstance();
try {
- modCallback = opDesc.getModificationOpCallbackFactory().createModificationOperationCallback(
+ modCallback = opDesc.getModificationOpCallbackFactory().createModificationOperationCallback(indexHelper.getResourceName(),
indexHelper.getResourceID(), index, ctx);
indexAccessor = index.createAccessor(modCallback, NoOpOperationCallback.INSTANCE);
ITupleFilterFactory tupleFilterFactory = opDesc.getTupleFilterFactory();
diff --git a/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java b/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java
index b3213d1..00ae8c2 100644
--- a/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java
+++ b/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java
@@ -30,20 +30,23 @@
import org.apache.hyracks.api.lifecycle.ILifeCycleComponent;
import org.apache.hyracks.storage.am.common.api.IIndex;
import org.apache.hyracks.storage.am.common.api.IIndexLifecycleManager;
+import org.apache.hyracks.storage.common.file.ILocalResourceRepository;
+import org.apache.hyracks.storage.common.file.LocalResource;
public class IndexLifecycleManager implements IIndexLifecycleManager, ILifeCycleComponent {
private static final long DEFAULT_MEMORY_BUDGET = 1024 * 1024 * 100; // 100 megabytes
private final Map<Long, IndexInfo> indexInfos;
private final long memoryBudget;
-
+ private final ILocalResourceRepository localResourcesRepository;
private long memoryUsed;
- public IndexLifecycleManager() {
- this(DEFAULT_MEMORY_BUDGET);
+ public IndexLifecycleManager(ILocalResourceRepository localResourcesRepository) {
+ this(localResourcesRepository, DEFAULT_MEMORY_BUDGET);
}
- public IndexLifecycleManager(long memoryBudget) {
+ public IndexLifecycleManager(ILocalResourceRepository localResourcesRepository, long memoryBudget) {
+ this.localResourcesRepository = localResourcesRepository;
this.indexInfos = new HashMap<Long, IndexInfo>();
this.memoryBudget = memoryBudget;
this.memoryUsed = 0;
@@ -65,66 +68,6 @@
info.isOpen = false;
return true;
- }
-
- @Override
- public IIndex getIndex(long resourceID) {
- IndexInfo info = indexInfos.get(resourceID);
- return info == null ? null : info.index;
- }
-
- @Override
- public void register(long resourceID, IIndex index) throws HyracksDataException {
- if (indexInfos.containsKey(resourceID)) {
- throw new HyracksDataException("Index with resource ID " + resourceID + " already exists.");
- }
-
- indexInfos.put(resourceID, new IndexInfo(index));
- }
-
- @Override
- public void unregister(long resourceID) throws HyracksDataException {
- IndexInfo info = indexInfos.remove(resourceID);
- if (info == null) {
- throw new HyracksDataException("Index with resource ID " + resourceID + " does not exist.");
- }
-
- if (info.referenceCount != 0) {
- indexInfos.put(resourceID, info);
- throw new HyracksDataException("Cannot remove index while it is open.");
- }
-
- if (info.isOpen) {
- info.index.deactivate();
- memoryUsed -= info.index.getMemoryAllocationSize();
- }
- }
-
- @Override
- public void open(long resourceID) throws HyracksDataException {
- IndexInfo info = indexInfos.get(resourceID);
- if (info == null) {
- throw new HyracksDataException("Failed to open index with resource ID " + resourceID
- + " since it does not exist.");
- }
-
- if (!info.isOpen) {
- long inMemorySize = info.index.getMemoryAllocationSize();
- while (memoryUsed + inMemorySize > memoryBudget) {
- if (!evictCandidateIndex()) {
- throw new HyracksDataException("Cannot activate index since memory budget would be exceeded.");
- }
- }
- info.index.activate();
- info.isOpen = true;
- memoryUsed += inMemorySize;
- }
- info.touch();
- }
-
- @Override
- public void close(long resourceID) {
- indexInfos.get(resourceID).untouch();
}
private class IndexInfo implements Comparable<IndexInfo> {
@@ -235,4 +178,79 @@
}
os.write(sb.toString().getBytes());
}
+
+ @Override
+ public void register(String resourceName, IIndex index) throws HyracksDataException {
+ long resourceID = getResourceID(resourceName);
+ if (indexInfos.containsKey(resourceID)) {
+ throw new HyracksDataException("Index with resource ID " + resourceID + " already exists.");
+ }
+
+ indexInfos.put(resourceID, new IndexInfo(index));
+ }
+
+ @Override
+ public void open(String resourceName) throws HyracksDataException {
+ long resourceID = getResourceID(resourceName);
+ IndexInfo info = indexInfos.get(resourceID);
+ if (info == null) {
+ throw new HyracksDataException("Failed to open index with resource ID " + resourceID
+ + " since it does not exist.");
+ }
+
+ if (!info.isOpen) {
+ long inMemorySize = info.index.getMemoryAllocationSize();
+ while (memoryUsed + inMemorySize > memoryBudget) {
+ if (!evictCandidateIndex()) {
+ throw new HyracksDataException("Cannot activate index since memory budget would be exceeded.");
+ }
+ }
+ info.index.activate();
+ info.isOpen = true;
+ memoryUsed += inMemorySize;
+ }
+ info.touch();
+ }
+
+ @Override
+ public void close(String resourceName) throws HyracksDataException {
+ long resourceID = getResourceID(resourceName);
+ indexInfos.get(resourceID).untouch();
+ }
+
+ @Override
+ public IIndex getIndex(String resourceName) throws HyracksDataException {
+ long resourceID = getResourceID(resourceName);
+ IndexInfo info = indexInfos.get(resourceID);
+ return info == null ? null : info.index;
+ }
+
+ @Override
+ public void unregister(String resourceName) throws HyracksDataException {
+ long resourceID = getResourceID(resourceName);
+ IndexInfo info = indexInfos.remove(resourceID);
+ if (info == null) {
+ throw new HyracksDataException("Index with resource ID " + resourceID + " does not exist.");
+ }
+
+ if (info.referenceCount != 0) {
+ indexInfos.put(resourceID, info);
+ throw new HyracksDataException("Cannot remove index while it is open.");
+ }
+
+ if (info.isOpen) {
+ info.index.deactivate();
+ memoryUsed -= info.index.getMemoryAllocationSize();
+ }
+ }
+
+ private long getResourceID(String resourceName) throws HyracksDataException {
+ LocalResource lr = localResourcesRepository.getResourceByName(resourceName);
+ return lr == null ? -1 : lr.getResourceId();
+ }
+
+ @Override
+ public IIndex getIndex(int datasetID, long resourceID) throws HyracksDataException {
+ throw new UnsupportedOperationException();
+ }
}
\ No newline at end of file
diff --git a/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/NoOpOperationCallbackFactory.java b/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/NoOpOperationCallbackFactory.java
index 4f855e4..076f7f8 100644
--- a/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/NoOpOperationCallbackFactory.java
+++ b/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/NoOpOperationCallbackFactory.java
@@ -19,6 +19,7 @@
package org.apache.hyracks.storage.am.common.impls;
import org.apache.hyracks.api.context.IHyracksTaskContext;
+import org.apache.hyracks.api.exceptions.HyracksDataException;
import org.apache.hyracks.storage.am.common.api.IModificationOperationCallback;
import org.apache.hyracks.storage.am.common.api.IModificationOperationCallbackFactory;
import org.apache.hyracks.storage.am.common.api.ISearchOperationCallback;
@@ -33,12 +34,13 @@
INSTANCE;
@Override
- public IModificationOperationCallback createModificationOperationCallback(long resourceId, Object resource, IHyracksTaskContext ctx) {
+ public ISearchOperationCallback createSearchOperationCallback(long resourceId, IHyracksTaskContext ctx) {
return NoOpOperationCallback.INSTANCE;
}
@Override
- public ISearchOperationCallback createSearchOperationCallback(long resourceId, IHyracksTaskContext ctx) {
+ public IModificationOperationCallback createModificationOperationCallback(String resourceName, long resourceId,
+ Object resource, IHyracksTaskContext ctx) throws HyracksDataException {
return NoOpOperationCallback.INSTANCE;
}
}
diff --git a/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java b/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
index 45522ce..0e23fa4 100644
--- a/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
+++ b/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
@@ -40,8 +40,7 @@
public ExternalBTreeDataflowHelper(IIndexOperatorDescriptor opDesc, IHyracksTaskContext ctx, int partition,
double bloomFilterFalsePositiveRate, ILSMMergePolicy mergePolicy,
ILSMOperationTrackerProvider opTrackerFactory, ILSMIOOperationScheduler ioScheduler,
- ILSMIOOperationCallbackFactory ioOpCallbackFactory, boolean needKeyDupCheck, int version,
- boolean durable) {
+ ILSMIOOperationCallbackFactory ioOpCallbackFactory, boolean needKeyDupCheck, int version, boolean durable) {
super(opDesc, ctx, partition, null, bloomFilterFalsePositiveRate, mergePolicy, opTrackerFactory, ioScheduler,
ioOpCallbackFactory, false, null, null, null, null, durable);
this.version = version;
@@ -50,8 +49,7 @@
public ExternalBTreeDataflowHelper(IIndexOperatorDescriptor opDesc, IHyracksTaskContext ctx, int partition,
List<IVirtualBufferCache> virtualBufferCaches, ILSMMergePolicy mergePolicy,
ILSMOperationTrackerProvider opTrackerFactory, ILSMIOOperationScheduler ioScheduler,
- ILSMIOOperationCallbackFactory ioOpCallbackFactory, boolean needKeyDupCheck, int version,
- boolean durable) {
+ ILSMIOOperationCallbackFactory ioOpCallbackFactory, boolean needKeyDupCheck, int version, boolean durable) {
this(opDesc, ctx, partition, DEFAULT_BLOOM_FILTER_FALSE_POSITIVE_RATE, mergePolicy, opTrackerFactory,
ioScheduler, ioOpCallbackFactory, needKeyDupCheck, version, durable);
}
@@ -61,14 +59,8 @@
if (index != null)
return index;
synchronized (lcManager) {
- long resourceID;
try {
- resourceID = getResourceID();
- } catch (HyracksDataException e) {
- return null;
- }
- try {
- index = lcManager.getIndex(resourceID);
+ index = lcManager.getIndex(resourceName);
} catch (HyracksDataException e) {
return null;
}
diff --git a/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java b/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java
index a5d4892..325ff91 100644
--- a/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java
+++ b/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java
@@ -48,8 +48,7 @@
public ExternalBTreeWithBuddyDataflowHelper(IIndexOperatorDescriptor opDesc, IHyracksTaskContext ctx,
int partition, double bloomFilterFalsePositiveRate, ILSMMergePolicy mergePolicy,
ILSMOperationTrackerProvider opTrackerFactory, ILSMIOOperationScheduler ioScheduler,
- ILSMIOOperationCallbackFactory ioOpCallbackFactory, int[] buddyBtreeFields, int version,
- boolean durable) {
+ ILSMIOOperationCallbackFactory ioOpCallbackFactory, int[] buddyBtreeFields, int version, boolean durable) {
super(opDesc, ctx, partition, null, bloomFilterFalsePositiveRate, mergePolicy, opTrackerFactory, ioScheduler,
ioOpCallbackFactory, null, null, null, durable);
this.buddyBtreeFields = buddyBtreeFields;
@@ -61,14 +60,8 @@
if (index != null)
return index;
synchronized (lcManager) {
- long resourceID;
try {
- resourceID = getResourceID();
- } catch (HyracksDataException e) {
- return null;
- }
- try {
- index = lcManager.getIndex(resourceID);
+ index = lcManager.getIndex(resourceName);
} catch (HyracksDataException e) {
return null;
}
diff --git a/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java b/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java
index e8c918a..b6f5941 100644
--- a/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java
+++ b/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java
@@ -75,14 +75,8 @@
if (index != null)
return index;
synchronized (lcManager) {
- long resourceID;
try {
- resourceID = getResourceID();
- } catch (HyracksDataException e) {
- return null;
- }
- try {
- index = lcManager.getIndex(resourceID);
+ index = lcManager.getIndex(resourceName);
} catch (HyracksDataException e) {
return null;
}
@@ -99,11 +93,11 @@
ITypeTraits[] filterTypeTraits, IBinaryComparatorFactory[] filterCmpFactories, int[] filterFields)
throws HyracksDataException {
try {
- return LSMRTreeUtils.createExternalRTree(file, diskBufferCache, diskFileMapProvider, typeTraits,
- rtreeCmpFactories, btreeCmpFactories, valueProviderFactories, rtreePolicyType,
- bloomFilterFalsePositiveRate, mergePolicy, opTracker, ioScheduler,
- ioOpCallbackFactory.createIOOperationCallback(), linearizeCmpFactory, btreeFields, version,
- durable);
+ return LSMRTreeUtils
+ .createExternalRTree(file, diskBufferCache, diskFileMapProvider, typeTraits, rtreeCmpFactories,
+ btreeCmpFactories, valueProviderFactories, rtreePolicyType, bloomFilterFalsePositiveRate,
+ mergePolicy, opTracker, ioScheduler, ioOpCallbackFactory.createIOOperationCallback(),
+ linearizeCmpFactory, btreeFields, version, durable);
} catch (TreeIndexException e) {
throw new HyracksDataException(e);
}
diff --git a/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ILocalResourceRepository.java b/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ILocalResourceRepository.java
index a826226..6f2e285 100644
--- a/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ILocalResourceRepository.java
+++ b/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ILocalResourceRepository.java
@@ -18,21 +18,15 @@
*/
package org.apache.hyracks.storage.common.file;
-import java.util.List;
-
import org.apache.hyracks.api.exceptions.HyracksDataException;
public interface ILocalResourceRepository {
-
- public LocalResource getResourceById(long id) throws HyracksDataException;
public LocalResource getResourceByName(String name) throws HyracksDataException;
public void insert(LocalResource resource) throws HyracksDataException;
- public void deleteResourceById(long id) throws HyracksDataException;
-
public void deleteResourceByName(String name) throws HyracksDataException;
- public List<LocalResource> getAllResources() throws HyracksDataException;
+ public long getMaxResourceID() throws HyracksDataException;
}
diff --git a/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactory.java b/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactory.java
index 3b5ea03..0adf998 100644
--- a/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactory.java
+++ b/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactory.java
@@ -30,8 +30,4 @@
public long createId() {
return id.getAndIncrement();
}
-
- public void initId(long id) {
- this.id.set(id);
- }
}
diff --git a/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactoryProvider.java b/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactoryProvider.java
index bbbc581..c30ba4d 100644
--- a/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactoryProvider.java
+++ b/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactoryProvider.java
@@ -18,8 +18,6 @@
*/
package org.apache.hyracks.storage.common.file;
-import java.util.List;
-
import org.apache.hyracks.api.exceptions.HyracksDataException;
public class ResourceIdFactoryProvider {
@@ -30,13 +28,7 @@
}
public ResourceIdFactory createResourceIdFactory() throws HyracksDataException {
- List<LocalResource> localResources = localResourceRepository.getAllResources();
- long largestResourceId = 0;
- for (LocalResource localResource : localResources) {
- if (largestResourceId < localResource.getResourceId()) {
- largestResourceId = localResource.getResourceId();
- }
- }
+ long largestResourceId = localResourceRepository.getMaxResourceID();
return new ResourceIdFactory(largestResourceId);
}
}
diff --git a/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/TransientLocalResourceRepository.java b/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/TransientLocalResourceRepository.java
index b3e9db0..3bc763d 100644
--- a/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/TransientLocalResourceRepository.java
+++ b/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/TransientLocalResourceRepository.java
@@ -18,9 +18,7 @@
*/
package org.apache.hyracks.storage.common.file;
-import java.util.ArrayList;
import java.util.HashMap;
-import java.util.List;
import java.util.Map;
import org.apache.hyracks.api.exceptions.HyracksDataException;
@@ -29,11 +27,6 @@
private Map<String, LocalResource> name2ResourceMap = new HashMap<String, LocalResource>();
private Map<Long, LocalResource> id2ResourceMap = new HashMap<Long, LocalResource>();
-
- @Override
- public LocalResource getResourceById(long id) throws HyracksDataException {
- return id2ResourceMap.get(id);
- }
@Override
public LocalResource getResourceByName(String name) throws HyracksDataException {
@@ -52,16 +45,6 @@
}
@Override
- public synchronized void deleteResourceById(long id) throws HyracksDataException {
- LocalResource resource = id2ResourceMap.get(id);
- if (resource == null) {
- throw new HyracksDataException("Resource doesn't exist");
- }
- id2ResourceMap.remove(id);
- name2ResourceMap.remove(resource.getResourceName());
- }
-
- @Override
public synchronized void deleteResourceByName(String name) throws HyracksDataException {
LocalResource resource = name2ResourceMap.get(name);
if (resource == null) {
@@ -72,11 +55,12 @@
}
@Override
- public List<LocalResource> getAllResources() throws HyracksDataException {
- List<LocalResource> resources = new ArrayList<LocalResource>();
- for (LocalResource resource : id2ResourceMap.values()) {
- resources.add(resource);
+ public long getMaxResourceID() throws HyracksDataException {
+ long maxResourceId = 0;
+
+ for (Long resourceId : id2ResourceMap.keySet()) {
+ maxResourceId = Math.max(maxResourceId, resourceId);
}
- return resources;
+ return maxResourceId;
}
}
diff --git a/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/test/support/TestStorageManagerComponentHolder.java b/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/test/support/TestStorageManagerComponentHolder.java
index d987501..671c479 100644
--- a/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/test/support/TestStorageManagerComponentHolder.java
+++ b/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/test/support/TestStorageManagerComponentHolder.java
@@ -75,7 +75,7 @@
public synchronized static IIndexLifecycleManager getIndexLifecycleManager(IHyracksTaskContext ctx) {
if (lcManager == null) {
- lcManager = new IndexLifecycleManager();
+ lcManager = new IndexLifecycleManager(getLocalResourceRepository(ctx));
}
return lcManager;
}
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 7
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Jenkins (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Jenkins has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 2:
Build Started https://asterix-jenkins.ics.uci.edu/job/hyracks-gerrit/412/
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: ASTERIXDB-1053: change IndexLifecycleManager API to use reso...
Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.
Change subject: ASTERIXDB-1053: change IndexLifecycleManager API to use resource name
......................................................................
Patch Set 5:
Build Started https://asterix-jenkins.ics.uci.edu/job/hyracks-gerrit/489/
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 5
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: ASTERIXDB-1053: change IndexLifecycleManager API to use reso...
Posted by "Till Westmann (Code Review)" <do...@asterixdb.incubator.apache.org>.
Till Westmann has posted comments on this change.
Change subject: ASTERIXDB-1053: change IndexLifecycleManager API to use resource name
......................................................................
Patch Set 5: Code-Review+1
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 5
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Yingyi Bu (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Yingyi Bu has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 4: Code-Review+2
LGTM.
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 4
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Till Westmann (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Till Westmann has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 2:
(1 comment)
Ok, so the goal of the change is to avoid keeping all the mappings (ID -> resource, name -> resource) in memory. Is that right?
If that's the goal, using Strings directly to identify files helps as we don't need to maintain the map.
However, digging though the code some it seems (if I dug correctly) that the resource identifiers are used for logging and recovery and so it seems that we still need to maintain the ids (and it's probably a good idea to not try to serialize strings into the log). So if we didn't want to have that mapping in memory we'd need to put it somewhere else (and it probably already is somewhere else, if it's used in the log).
Is that right as well (or didn't I look closely enough at the code)?
In that case it seems that it'd be difficult to use strings for everything,
https://asterix-gerrit.ics.uci.edu/#/c/343/2/hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
File hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java:
Line 66: index = lcManager.getIndex(file.getFile().getPath());
> Other DataflowHelpers don't override this method. Therefore, they all go th
That sounds like a good idea. Also, we could probably streamline the method a bit and reduce the number of try-catch-blocks.
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: Yes
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Murtadha Hubail (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Hello abdullah alamoudi, Jenkins,
I'd like you to reexamine a change. Please visit
https://asterix-gerrit.ics.uci.edu/343
to look at the new patch set (#3).
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Add support for persistent local resources to IndexLifecycleManager
Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
---
M hyracks/hyracks-examples/btree-example/btreehelper/src/main/java/org/apache/hyracks/examples/btree/helper/RuntimeContext.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexLifecycleManager.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/NoOpOperationCallbackFactory.java
M hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
M hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java
M hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ILocalResourceRepository.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactory.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactoryProvider.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/TransientLocalResourceRepository.java
M hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/test/support/TestStorageManagerComponentHolder.java
14 files changed, 145 insertions(+), 146 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/hyracks refs/changes/43/343/3
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 3
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "abdullah alamoudi (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
abdullah alamoudi has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Jenkins (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Jenkins has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 4:
Build Started https://asterix-jenkins.ics.uci.edu/job/hyracks-gerrit/432/
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 4
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Yingyi Bu (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Yingyi Bu has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 2:
(3 comments)
I leave some comments in the code based on my understanding. But I could be wrong. Just correct me if I'm wrong:-)
https://asterix-gerrit.ics.uci.edu/#/c/343/2/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IIndexLifecycleManager.java
File hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IIndexLifecycleManager.java:
Line 34: public boolean supportsPersistentLocalResources();
s/supports/support ?
Add some comments to clarify what does supportPersistentLocalResources() mean?
Does it mean all the resource look-ups will be based on names if that returns true?
It looks all methods are duplicated...
Could the parameter be "ResourceHandle", where internally, a ResourceHandle can contain a string and long? Then, at runtime, if the long presents, use it to lookup, otherwise (or the long id doesn't find anything) use the string.
https://asterix-gerrit.ics.uci.edu/#/c/343/2/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java
File hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java:
Line 27: Object resource, IHyracksTaskContext ctx) throws HyracksDataException;
Why "Object resource" is necessary here?
It seems they are never used in the implementations?
(It is a problem of the old code though..)
https://asterix-gerrit.ics.uci.edu/#/c/343/2/hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
File hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java:
Line 66: index = lcManager.getIndex(file.getFile().getPath());
Could the method be moved up to the super class?
It looks this code is duplicated in several External*TreeDataflowHelper classes:
ExternalBTreeDataflowHelper, ExternalBTreeWithBuddyDataflowHelper, and ExternalRTreeDataflowHelper.java.
Also, why only those External*TreeDataFlowHelpers have this change?
How about other *DataflowHelpers?
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: Yes
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Jenkins (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Jenkins has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 3: Verified+1
Build Successful
https://asterix-jenkins.ics.uci.edu/job/hyracks-gerrit/431/ : SUCCESS
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 3
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Jenkins (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Jenkins has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 4: Verified+1
Build Successful
https://asterix-jenkins.ics.uci.edu/job/hyracks-gerrit/432/ : SUCCESS
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 4
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Yingyi Bu (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Yingyi Bu has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 3: Code-Review+1
(5 comments)
LGTM. Just a few minor comments.
https://asterix-gerrit.ics.uci.edu/#/c/343/3/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
File hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java:
Line 59: index = lcManager.getIndex(file.getFile().getPath());
create a local variable for "file.getFile().getPath()"?
https://asterix-gerrit.ics.uci.edu/#/c/343/3/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java
File hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java:
Line 60: index = lcManager.getIndex(file.getFile().getPath());
create a local variable for "file.getFile().getPath()"?
https://asterix-gerrit.ics.uci.edu/#/c/343/3/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java
File hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java:
Line 75: index = lcManager.getIndex(file.getFile().getPath());
create a local variable for "file.getFile().getPath()"?
https://asterix-gerrit.ics.uci.edu/#/c/343/3/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ILocalResourceRepository.java
File hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ILocalResourceRepository.java:
Line 21: public LocalResource getResourceById(long id) throws HyracksDataException;
Do we still need this? Because your Asterix CL throws unsupported exception for this method.
Line 28: public void deleteResourceById(long id) throws HyracksDataException;
Do we still need this? Because your Asterix CL throws unsupported exception for this method.
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 3
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: Yes
Change in hyracks[master]: ASTERIXDB-1053: change IndexLifecycleManager API to use reso...
Posted by "Yingyi Bu (Code Review)" <do...@asterixdb.incubator.apache.org>.
Yingyi Bu has posted comments on this change.
Change subject: ASTERIXDB-1053: change IndexLifecycleManager API to use resource name
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 6
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: ASTERIXDB-1053: change IndexLifecycleManager API to use reso...
Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.
Change subject: ASTERIXDB-1053: change IndexLifecycleManager API to use resource name
......................................................................
Patch Set 5: Verified+1
Build Successful
https://asterix-jenkins.ics.uci.edu/job/hyracks-gerrit/489/ : SUCCESS
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 5
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Murtadha Hubail (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Murtadha Hubail has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 2:
The best option would be to eliminate the APIs with resourceId. However, I cannot do that since there might be some other system already using these APIs. Do you have any suggestion in mind?
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Yingyi Bu (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Yingyi Bu has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 2:
(1 comment)
https://asterix-gerrit.ics.uci.edu/#/c/343/2/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IIndexLifecycleManager.java
File hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IIndexLifecycleManager.java:
Line 37:
I see that your asterixdb change only uses resource names... I'm ok to only keep the string-based methods if it doesn't cause perf. problems.
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: Yes
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Till Westmann (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Till Westmann has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 4
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Murtadha Hubail (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Murtadha Hubail has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 2:
(4 comments)
https://asterix-gerrit.ics.uci.edu/#/c/343/2/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IIndexLifecycleManager.java
File hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IIndexLifecycleManager.java:
Line 34: public boolean supportsPersistentLocalResources();
> s/supports/support ?
It means that its LocalResourceRepository is of type PersistentLocalResourceRepository.
Line 37:
> I see that your asterixdb change only uses resource names... I'm ok to onl
I would like to keep the string only as well, but I don't want to remove the other methods from the interface incase some system is using them via TransientLocalResourceRepository. You may want to have a look at my discussion with Till. In both cases (Long and String), they are accessed using hashCode(), I doubt there will be performance problems.
https://asterix-gerrit.ics.uci.edu/#/c/343/2/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java
File hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java:
Line 27: Object resource, IHyracksTaskContext ctx) throws HyracksDataException;
> Why "Object resource" is necessary here?
Yes, I don't think it is used.
https://asterix-gerrit.ics.uci.edu/#/c/343/2/hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
File hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java:
Line 66: index = lcManager.getIndex(file.getFile().getPath());
> Could the method be moved up to the super class?
Other DataflowHelpers don't override this method. Therefore, they all go thru IndexDataflowHelper. There is no super class for the External indexes, they extend the same class (AbstractLSMIndexDataflowHelper) as the internal indexes. A super class could be created for the external indexes to factor out this method.
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: Yes
Change in hyracks[master]: ASTERIXDB-1053: change IndexLifecycleManager API to use reso...
Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.
Change subject: ASTERIXDB-1053: change IndexLifecycleManager API to use resource name
......................................................................
Patch Set 6:
Build Started https://asterix-jenkins.ics.uci.edu/job/hyracks-gerrit/493/
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 6
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: ASTERIXDB-1053: change IndexLifecycleManager API to use reso...
Posted by "Murtadha Hubail (Code Review)" <do...@asterixdb.incubator.apache.org>.
Hello abdullah alamoudi, Yingyi Bu, Till Westmann, Jenkins,
I'd like you to reexamine a change. Please visit
https://asterix-gerrit.ics.uci.edu/343
to look at the new patch set (#5).
Change subject: ASTERIXDB-1053: change IndexLifecycleManager API to use resource name
......................................................................
ASTERIXDB-1053: change IndexLifecycleManager API to use resource name
Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
---
M hyracks/hyracks-examples/btree-example/btreehelper/src/main/java/org/apache/hyracks/examples/btree/helper/RuntimeContext.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexDataflowHelper.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexLifecycleManager.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IModificationOperationCallbackFactory.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexInsertUpdateDeleteOperatorNodePushable.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java
M hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/NoOpOperationCallbackFactory.java
M hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
M hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeWithBuddyDataflowHelper.java
M hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/dataflow/ExternalRTreeDataflowHelper.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ILocalResourceRepository.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactory.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/ResourceIdFactoryProvider.java
M hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/TransientLocalResourceRepository.java
M hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/test/support/TestStorageManagerComponentHolder.java
16 files changed, 150 insertions(+), 184 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/hyracks refs/changes/43/343/5
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 5
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Till Westmann (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Till Westmann has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 3: Code-Review+1
(6 comments)
Look much better now (and I learned a lot), thanks!
My only remaining comments are about small code cleanups that would make things a littler easier to read (at least for me).
I'll leave the question about other users of the interface to Yingyi.
https://asterix-gerrit.ics.uci.edu/#/c/343/3/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
File hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java:
Line 71: long resourceID = getResourceID();
Could we move the declaration of resourceID down to the place it is actually needed (inside the try-block)?
Line 73: index = lcManager.getIndex(file.getFile().getPath());
Could we create a local variable for "file.getFile().getPath()"?
Line 104: long resourceID = getResourceID();
Maybe we can inline resourceID?
Line 110: index = lcManager.getIndex(file.getFile().getPath());
Could we create a local variable for "file.getFile().getPath()"?
Line 130: index = lcManager.getIndex(file.getFile().getPath());
Could we create a local variable for "file.getFile().getPath()"?
https://asterix-gerrit.ics.uci.edu/#/c/343/3/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java
File hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java:
Line 253: return lr.getResourceId();
return lr == null ? -1 : lr.getResourceId() ? :)
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 3
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: Yes
Change in hyracks[master]: ASTERIXDB-1053: change IndexLifecycleManager API to use reso...
Posted by "Jenkins (Code Review)" <do...@asterixdb.incubator.apache.org>.
Jenkins has posted comments on this change.
Change subject: ASTERIXDB-1053: change IndexLifecycleManager API to use resource name
......................................................................
Patch Set 6: Verified+1
Build Successful
https://asterix-jenkins.ics.uci.edu/job/hyracks-gerrit/493/ : SUCCESS
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 6
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Murtadha Hubail (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Murtadha Hubail has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 3:
(6 comments)
Thanks Till. I addressed your comments. I will wait for Yingyi's before submitting the new patch.
https://asterix-gerrit.ics.uci.edu/#/c/343/3/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java
File hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexDataflowHelper.java:
Line 71: long resourceID = getResourceID();
> Could we move the declaration of resourceID down to the place it is actuall
it is needed before the try-block to check if there is an old resource id for this index. I moved it closer to that check.
Line 73: index = lcManager.getIndex(file.getFile().getPath());
> Could we create a local variable for "file.getFile().getPath()"?
Done
Line 104: long resourceID = getResourceID();
> Maybe we can inline resourceID?
Done
Line 110: index = lcManager.getIndex(file.getFile().getPath());
> Could we create a local variable for "file.getFile().getPath()"?
Done
Line 130: index = lcManager.getIndex(file.getFile().getPath());
> Could we create a local variable for "file.getFile().getPath()"?
Done
https://asterix-gerrit.ics.uci.edu/#/c/343/3/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java
File hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexLifecycleManager.java:
Line 253: return lr.getResourceId();
> return lr == null ? -1 : lr.getResourceId() ? :)
Done
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 3
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: Yes
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "Murtadha Hubail (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
Murtadha Hubail has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 3:
@Till and @Yingyi,
I removed the API using resource id and the checks for persistent local resources. Please have a look at the change now. If you are happy with it, I will submit the Asterix side based on this change. Otherwise, I will revert these changes and return the checks.
Note that the deprecated methods on ILocalResourceRepository can be removed since no one is using them (at least in hyracks and Asterix).
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 3
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: No
Change in hyracks[master]: Add support for persistent local resources to IndexLifecycle...
Posted by "abdullah alamoudi (Code Review)" <do...@asterix-gerrit.ics.uci.edu>.
abdullah alamoudi has posted comments on this change.
Change subject: Add support for persistent local resources to IndexLifecycleManager
......................................................................
Patch Set 2:
(1 comment)
https://asterix-gerrit.ics.uci.edu/#/c/343/2/hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java
File hyracks/hyracks-storage-am-lsm-btree/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/btree/dataflow/ExternalBTreeDataflowHelper.java:
Line 66: index = lcManager.getIndex(file.getFile().getPath());
> That sounds like a good idea. Also, we could probably streamline the method
This is not as easy as it looks since these indexes don't have much in common except that they are external. Having an abstract class on top of them will create a lot of unneeded complexity.
But anyone is welcome to take a shot at it :-)
--
To view, visit https://asterix-gerrit.ics.uci.edu/343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: Yes