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 2021/03/13 04:04:24 UTC
[GitHub] [iotdb] weizihan0110 opened a new pull request #2828: [ISSUE-2827] add a new method in MNode.
weizihan0110 opened a new pull request #2828:
URL: https://github.com/apache/iotdb/pull/2828
the related issue is at: https://github.com/apache/iotdb/issues/2827
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] jixuan1989 merged pull request #2828: [ISSUE-2827] add an overrided addChild() method in MNode.
Posted by GitBox <gi...@apache.org>.
jixuan1989 merged pull request #2828:
URL: https://github.com/apache/iotdb/pull/2828
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] jixuan1989 commented on a change in pull request #2828: [ISSUE-2827] add a new method in MNode.
Posted by GitBox <gi...@apache.org>.
jixuan1989 commented on a change in pull request #2828:
URL: https://github.com/apache/iotdb/pull/2828#discussion_r594068888
##########
File path: server/src/main/java/org/apache/iotdb/db/metadata/mnode/MNode.java
##########
@@ -102,6 +102,36 @@ public void addChild(String name, MNode child) {
children.putIfAbsent(name, child);
}
+ /**
+ * Add a child to the current mnode.
+ *
+ * <p>This method will not take the child's name as one of the inputs and will also make this
+ * Mnode be child node's parent. All is to reduce the probability of mistaken by users and be more
+ * convenient for users to use. And the return of this method is used to conveniently construct a
+ * chain of time series for users.
+ *
+ * @param child child's node
+ * @return return the MNode already added
+ */
+ MNode addChild(MNode child) {
+ /* use cpu time to exchange memory
+ * measurementNode's children should be null to save memory
+ * add child method will only be called when writing MTree, which is not a frequent operation
+ */
+ if (children == null) {
+ // double check, children is volatile
+ synchronized (this) {
+ if (children == null) {
+ children = new ConcurrentHashMap<>();
+ }
+ }
+ }
+
+ children.putIfAbsent(child.getName(), child);
+ child.parent = this;
Review comment:
Besides, if you set child.parent in this method, better to set its parent before putting it in children map.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] jixuan1989 commented on a change in pull request #2828: [ISSUE-2827] add a new method in MNode.
Posted by GitBox <gi...@apache.org>.
jixuan1989 commented on a change in pull request #2828:
URL: https://github.com/apache/iotdb/pull/2828#discussion_r594068674
##########
File path: server/src/main/java/org/apache/iotdb/db/metadata/mnode/MNode.java
##########
@@ -102,6 +102,36 @@ public void addChild(String name, MNode child) {
children.putIfAbsent(name, child);
}
+ /**
+ * Add a child to the current mnode.
+ *
+ * <p>This method will not take the child's name as one of the inputs and will also make this
+ * Mnode be child node's parent. All is to reduce the probability of mistaken by users and be more
+ * convenient for users to use. And the return of this method is used to conveniently construct a
+ * chain of time series for users.
+ *
+ * @param child child's node
+ * @return return the MNode already added
+ */
+ MNode addChild(MNode child) {
+ /* use cpu time to exchange memory
+ * measurementNode's children should be null to save memory
+ * add child method will only be called when writing MTree, which is not a frequent operation
+ */
+ if (children == null) {
+ // double check, children is volatile
+ synchronized (this) {
+ if (children == null) {
+ children = new ConcurrentHashMap<>();
+ }
+ }
+ }
+
+ children.putIfAbsent(child.getName(), child);
+ child.parent = this;
Review comment:
this is inconsist with existing addChild(String name, MNode node);
Either set child.parent in the method or not.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org