You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2023/01/03 14:17:56 UTC
[GitHub] [iotdb] Cpaulyz opened a new pull request, #8721: [IOTDB-5342] Separate filtering and business logic in AbstractTreeVisitor
Cpaulyz opened a new pull request, #8721:
URL: https://github.com/apache/iotdb/pull/8721
## Description
1. Separate filtering and business logic in AbstractTreeVisitor.
```
/**
* Internal-match means the node matches an internal node name of the given path pattern. root.sg
* internal match root.sg.**(pattern). This method should be implemented according to concrete
* tasks.
*
* <p>Return whether the subtree of given node should be processed. If return true, the traversing
* process will keep traversing the subtree. If return false, the traversing process will skip the
* subtree of given node.
*/
protected abstract boolean processInternalMatchedNode(N node) throws Exception;
/**
* Full-match means the node matches the last node name of the given path pattern. root.sg.d full
* match root.sg.**(pattern) This method should be implemented according to concrete tasks.
*
* <p>Return whether the subtree of given node should be processed. If return true, the traversing
* process will keep traversing the subtree. If return false, the traversing process will skip the
* subtree of given node.
*/
protected abstract boolean processFullMatchedNode(N node) throws Exception;
/** Only accepted nodes will be considered for hasNext() and next() */
protected abstract boolean acceptNode(N node) throws Exception;
```
2. Injecting logic for releasing node resources in the AbstractTreeVistor framework. In ClusterSchemaTreeTest, a Mock test class `MockSchemaTreeMeasurementVisitor` is used to simulate the pin and unpin process to test this releasing logic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] MarcosZyk commented on a diff in pull request #8721: [IOTDB-5342] Separate filtering and business logic in AbstractTreeVisitor
Posted by GitBox <gi...@apache.org>.
MarcosZyk commented on code in PR #8721:
URL: https://github.com/apache/iotdb/pull/8721#discussion_r1061057693
##########
node-commons/src/main/java/org/apache/iotdb/commons/schema/tree/AbstractTreeVisitor.java:
##########
@@ -189,17 +195,26 @@ protected void getNext() throws Exception {
continue;
}
- node = iterator.next();
+ nextTempNode = iterator.next();
if (currentStateMatchInfo.hasFinalState()) {
- shouldVisitSubtree = processFullMatchedNode(node);
+ if (acceptNode(nextTempNode)) {
+ nextMatchedNode = nextTempNode;
+ }
+ shouldVisitSubtree = processFullMatchedNode(nextTempNode);
} else {
- shouldVisitSubtree = processInternalMatchedNode(node);
+ shouldVisitSubtree = processInternalMatchedNode(nextTempNode);
Review Comment:
Maybe we should set up the following abstract interfaces:
1. ```acceptInternalMatchedNode``` and ```acceptFullMatchedNode```
2. ```shouldVisitSubtreeOfInternalMatchedNode``` and ```shouldVisitSubtreeOfFullMatchedNode```
##########
node-commons/src/main/java/org/apache/iotdb/commons/schema/tree/AbstractTreeVisitor.java:
##########
@@ -274,11 +292,12 @@ protected final N getParentOfNextMatchedNode() {
// Get an iterator of all children.
protected abstract Iterator<N> getChildrenIterator(N parent) throws Exception;
- // Release a child with the given childName.
- protected void releaseChild(N child) {}
+ // Release a child node.
+ protected void releaseNode(N node) {}
- // Release an iterator of all children.
- protected void releaseChildrenIterator(Iterator<N> childrenIterator) {}
+ // Release an iterator. It is not necessary to deal with all the elements in the iterator.
+ // Only the elements that have been fetched but not returned by next() need to be released.
+ protected void releaseNodeIterator(Iterator<N> nodeIterator) {}
Review Comment:
seems no invokation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] MarcosZyk merged pull request #8721: [IOTDB-5342][IOTDB-5359][IOTDB-5360] Some optimization in AbstractTreeVisitor
Posted by GitBox <gi...@apache.org>.
MarcosZyk merged PR #8721:
URL: https://github.com/apache/iotdb/pull/8721
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org