Luo Chen has posted comments on this change.

Change subject: [ASTERIXDB-2231][STO] Separate primary op tracker for each 
partition
......................................................................


Patch Set 19:

(8 comments)

https://asterix-gerrit.ics.uci.edu/#/c/2263/15/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/dataflow/ComponentRollbackTest.java
File 
asterixdb/asterix-app/src/test/java/org/apache/asterix/test/dataflow/ComponentRollbackTest.java:

PS15, Line 105:        txnCtx = nc.getTransactionManager().be
> why 1 when there's use of partition 0 in the index creation?
I don't remember why I choose 1 instead of 0 here...Changed it back.


https://asterix-gerrit.ics.uci.edu/#/c/2263/19/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/api/INcApplicationContext.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/api/INcApplicationContext.java:

PS19, Line 69: getLSMBTreeOperationTracker
> Let's rename this method too. maybe getPrimaryOperationTracker
Done. But this method hasn't been used anymore...Maybe this should be removed 
in the future...


https://asterix-gerrit.ics.uci.edu/#/c/2263/15/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetInfo.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetInfo.java:

PS15, Line 158: public synchronized void addIndex(long resourceID, IndexInfo 
indexInfo) {
              :         indexes.put(resourceID, indexInfo);
              :         
partitionIndexes.computeIfAbsent(indexInfo.getPartition(), partition -> new 
HashSet<>()).add(indexInfo);
              :     }
> Wait, how'd this get mapped in before without this method and its complemen
Previously the caller simply call getIndexes() to get the original map, and 
modifies it in-place...(which I think is not a good idea)


https://asterix-gerrit.ics.uci.edu/#/c/2263/18/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetInfo.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetInfo.java:

PS18, Line 76: etDatasetPartitionIn
> let's rename to getDatasetPartitionOpenIndexes ? since this only cares abou
Done


https://asterix-gerrit.ics.uci.edu/#/c/2263/15/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetLifecycleManager.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetLifecycleManager.java:

PS15, Line 221: }
              :         if (dsr.getDatasetInfo().getReferenceCount() != 0 || 
!dsr.getDatasetInfo().isOpen()
              :                 || dsr.isMetadataDataset()) {
              :             r
> is there some way that the number of active operations could increment on a
I changed the order of checks back to the original to make sure this sticks to 
the original semantics..
But even for the previous code, I think it's still possible? Since this code is 
not synchronized, it's possible that after all condition passes, then a 
transaction tries to touch a dataset?


Line 441:                         //notification will come from LogBuffer class 
(notifyFlushTerminator)
> idk why sonar says this is critical but the rule says minor?
I saw this warning...But we cannot simply put it into a while loop waiting for 
isFlush, because for a flush log record, we didn't set isFlush to false 
previously...
So I just stick to the original logic.


https://asterix-gerrit.ics.uci.edu/#/c/2263/18/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetLifecycleManager.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetLifecycleManager.java:

Line 443:                     } catch (InterruptedException e) {
> +1
Done


PS18, Line 620: public int getResourcePartition(String resourcePath) throws 
HyracksDataException {
              :         // parse partition from the resource path
              :         // NOTE: we cannot get partition from 
resourceRepository, since this method could be called during
              :         // an index resource is being created
              :         return 
StoragePathUtil.getPartitionNumFromRelativePath(resourcePath);
              :     }
> why not remove this from the interface and simply call the Util?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9eb3854d2343e45beeccb87b0d434e5f4efd69c9
Gerrit-PatchSet: 19
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Luo Chen <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Luo Chen <[email protected]>
Gerrit-Reviewer: Murtadha Hubail <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to