[GitHub] [incubator-iotdb] HTHou commented on a change in pull request #464: Modified Decoder and SequenceReader to support old version of TsFile
HTHou commented on a change in pull request #464: Modified Decoder and
SequenceReader to support old version of TsFile
URL: https://github.com/apache/incubator-iotdb/pull/464#discussion_r337331915
##
File path:
tsfile/src/main/java/org/apache/iotdb/tsfile/file/metadata/TsFileMetaData.java
##
@@ -150,9 +150,11 @@ public static TsFileMetaData deserializeFrom(ByteBuffer
buffer) throws IOExcepti
if (ReadWriteIOUtils.readIsNull(buffer)) {
fileMetaData.createdBy = ReadWriteIOUtils.readString(buffer);
-}
-fileMetaData.totalChunkNum = ReadWriteIOUtils.readInt(buffer);
-fileMetaData.invalidChunkNum = ReadWriteIOUtils.readInt(buffer);
+}
+// if using v0.8.0 TsFile, use 0 to represent missing fields
+fileMetaData.totalChunkNum = buffer == null ? 0 :
ReadWriteIOUtils.readInt(buffer);
+fileMetaData.invalidChunkNum = buffer == null ? 0 :
ReadWriteIOUtils.readInt(buffer);
Review comment:
> Could this buffer really be null?
Thank you. I've understood this point.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] HTHou commented on a change in pull request #464: Modified Decoder and SequenceReader to support old version of TsFile
HTHou commented on a change in pull request #464: Modified Decoder and
SequenceReader to support old version of TsFile
URL: https://github.com/apache/incubator-iotdb/pull/464#discussion_r337007967
##
File path:
tsfile/src/main/java/org/apache/iotdb/tsfile/file/metadata/TsFileMetaData.java
##
@@ -150,9 +150,11 @@ public static TsFileMetaData deserializeFrom(ByteBuffer
buffer) throws IOExcepti
if (ReadWriteIOUtils.readIsNull(buffer)) {
fileMetaData.createdBy = ReadWriteIOUtils.readString(buffer);
-}
-fileMetaData.totalChunkNum = ReadWriteIOUtils.readInt(buffer);
-fileMetaData.invalidChunkNum = ReadWriteIOUtils.readInt(buffer);
+}
+// if using v0.8.0 TsFile, use 0 to represent missing fields
+fileMetaData.totalChunkNum = buffer == null ? 0 :
ReadWriteIOUtils.readInt(buffer);
+fileMetaData.invalidChunkNum = buffer == null ? 0 :
ReadWriteIOUtils.readInt(buffer);
Review comment:
> Could this buffer really be null?
Did you mean I should change `buffer == null`to
`!ReadWriteIOUtils.readIsNull(buffer)` ?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jixuan1989 commented on a change in pull request #465: [IOTDB-261]Check path validity in session
jixuan1989 commented on a change in pull request #465: [IOTDB-261]Check path
validity in session
URL: https://github.com/apache/incubator-iotdb/pull/465#discussion_r337327371
##
File path: session/src/test/java/org/apache/iotdb/session/IoTDBSessionIT.java
##
@@ -280,4 +280,59 @@ private void insert_via_sql() throws TException,
IoTDBRPCException {
session.executeNonQueryStatement(
"insert into root.sg1.d1(timestamp,s1, s2, s3) values(100, 1,2,3)");
}
+
+ @Test
+ public void checkPathTest()
+ throws ClassNotFoundException, SQLException, IoTDBSessionException,
TException, IoTDBRPCException {
+session = new Session("127.0.0.1", 6667, "root", "root");
+session.open();
+
+//test set sg
+checkSetSG(session, "root.vehicle", true);
+checkSetSG(session, "root.123456", true);
+checkSetSG(session, "root._1234", true);
+checkSetSG(session, "root._vehicle", true);
+checkSetSG(session, "root.\tvehicle", false);
Review comment:
how about `"root.\\tvehicle"`
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jixuan1989 commented on a change in pull request #465: [IOTDB-261]Check path validity in session
jixuan1989 commented on a change in pull request #465: [IOTDB-261]Check path
validity in session
URL: https://github.com/apache/incubator-iotdb/pull/465#discussion_r337327371
##
File path: session/src/test/java/org/apache/iotdb/session/IoTDBSessionIT.java
##
@@ -280,4 +280,59 @@ private void insert_via_sql() throws TException,
IoTDBRPCException {
session.executeNonQueryStatement(
"insert into root.sg1.d1(timestamp,s1, s2, s3) values(100, 1,2,3)");
}
+
+ @Test
+ public void checkPathTest()
+ throws ClassNotFoundException, SQLException, IoTDBSessionException,
TException, IoTDBRPCException {
+session = new Session("127.0.0.1", 6667, "root", "root");
+session.open();
+
+//test set sg
+checkSetSG(session, "root.vehicle", true);
+checkSetSG(session, "root.123456", true);
+checkSetSG(session, "root._1234", true);
+checkSetSG(session, "root._vehicle", true);
+checkSetSG(session, "root.\tvehicle", false);
Review comment:
how about root.\\tvehicle
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] Genius-pig edited a comment on issue #458: [IOTDB-165][TsFile] Fix a example
Genius-pig edited a comment on issue #458: [IOTDB-165][TsFile] Fix a example
URL: https://github.com/apache/incubator-iotdb/pull/458#issuecomment-544153656
## why I delete `position()` method?
1. This was requested change came from @jt2594838. @qiaojialin and I talked
personally, then we decided to delete it.
2. This was literally because every time when you new a
`TsFileSequenceReader` object and its constructor would call
`loadMetadataSize()` method, `loadMetadataSize()` would call `position()`
method. In local mode, it actually called `FileChannel.position()` method. As
we know, it was a method would move the position pointer. So questions were
**Why to move the position pointer? It is really necessary to move the position
pointer when I new a `TsFileSequenceReader` object?**
## why to move the position pointer?
Let's firstly check out TsFile structure.
https://user-images.githubusercontent.com/26118649/67146402-72c89700-f2bd-11e9-8f09-112a8456799b.png;>
You can find `MetaMarker.CHUNK_HEADER=0x1` after Magic String. There is a
method called `readMarker()` to read `MetaMarker.CHUNK_HEADER=0x1`.
```
/**
* read one byte from the input. this method is not thread safe
*/
public byte readMarker() throws IOException {
markerBuffer.clear();
if (ReadWriteIOUtils.readAsPossible(tsFileInput, markerBuffer) == 0) {
throw new IOException("reach the end of the file.");
}
markerBuffer.flip();
return markerBuffer.get();
}
```
There is no `position()` method in `readMarker()`. So it used to use
`position()` in `loadMetadataSize()` to move the position pointer. But there
are many tests and examples use `readMarker()`. After you remove `position()`
in `loadMetadataSize()`, you have to use `position()` before `readMarker()`.
@qiaojialin suggested me do that and I just done it.
## It is really necessary to move the position pointer when I new a
`TsFileSequenceReader` object?
No. Move the position pointer when you really need it.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 merged pull request #440: reconstruct antlrv3 grammar to improve performance
jt2594838 merged pull request #440: reconstruct antlrv3 grammar to improve performance URL: https://github.com/apache/incubator-iotdb/pull/440 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on issue #435: [IOTDB-174] Fix querying timeseries interface cannot make a query by the specified path prefix
jt2594838 commented on issue #435: [IOTDB-174] Fix querying timeseries interface cannot make a query by the specified path prefix URL: https://github.com/apache/incubator-iotdb/pull/435#issuecomment-544787183 There are failing tests. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 merged pull request #431: display cache hit rate through jconsole
jt2594838 merged pull request #431: display cache hit rate through jconsole URL: https://github.com/apache/incubator-iotdb/pull/431 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 merged pull request #439: Remove jdk constrain of jdk8 and 11
jt2594838 merged pull request #439: Remove jdk constrain of jdk8 and 11 URL: https://github.com/apache/incubator-iotdb/pull/439 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #430: [IOTDB-193]Create schema automatically
jt2594838 commented on a change in pull request #430: [IOTDB-193]Create schema
automatically
URL: https://github.com/apache/incubator-iotdb/pull/430#discussion_r337312898
##
File path: server/src/main/java/org/apache/iotdb/db/metadata/MManager.java
##
@@ -556,6 +556,8 @@ private String deletePathFromMTree(String path)
public void setStorageGroupToMTree(String path) throws
MetadataErrorException {
lock.writeLock().lock();
try {
+ checkAndGetDataTypeCache.clear();
+ mNodeCache.clear();
Review comment:
Is it necessary to clear the cache when you are doing an insertion?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on issue #429: [IOTDB-205]Support storage-group-level data ttl
jt2594838 commented on issue #429: [IOTDB-205]Support storage-group-level data ttl URL: https://github.com/apache/incubator-iotdb/pull/429#issuecomment-544778580 Creating a new inner class object or creating a lambda does not seem quite different to me. But I hope using method reference could help a bit. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #429: [IOTDB-205]Support storage-group-level data ttl
jt2594838 commented on a change in pull request #429: [IOTDB-205]Support
storage-group-level data ttl
URL: https://github.com/apache/incubator-iotdb/pull/429#discussion_r337307205
##
File path:
server/src/main/java/org/apache/iotdb/db/metadata/MetadataOperationType.java
##
@@ -34,5 +34,6 @@ private MetadataOperationType(){
public static final String UNLINK_MNODE_FROM_PTREE = "7";
public static final String ADD_INDEX_TO_PATH = "8";
public static final String DELETE_INDEX_FROM_PATH = "9";
- public static final String DELETE_STORAGE_GROUP_FROM_MTREE = "10";
+ public static final String SET_TTL = "10";
+ public static final String DELETE_STORAGE_GROUP_FROM_MTREE = "11";
Review comment:
Because when I created the field SET_TTL, there was no such field
DELETE_STORAGE_GROUP_FROM_MTREE. And later I merged master which brought
DELETE_STORAGE_GROUP_FROM_MTREE.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #429: [IOTDB-205]Support storage-group-level data ttl
jt2594838 commented on a change in pull request #429: [IOTDB-205]Support
storage-group-level data ttl
URL: https://github.com/apache/incubator-iotdb/pull/429#discussion_r337308720
##
File path:
server/src/main/java/org/apache/iotdb/db/engine/storagegroup/TsFileProcessor.java
##
@@ -576,6 +579,8 @@ public String getStorageGroupName() {
QueryUtils.modifyChunkMetaData(chunkMetaDataList,
modifications);
+ chunkMetaDataList.removeIf(chunkMetaData -> chunkMetaData.getEndTime() <
timeLowerBound);
Review comment:
This will require maintaining a ThreadLocal predicate for each query. As far
as I am concerned, this works no different from using a lambda since new object
construction is unavoidable.
Somehow, I moved `timeLowerBound` into `QueryContext` and replaced this with
a method reference.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #429: [IOTDB-205]Support storage-group-level data ttl
jt2594838 commented on a change in pull request #429: [IOTDB-205]Support
storage-group-level data ttl
URL: https://github.com/apache/incubator-iotdb/pull/429#discussion_r337307558
##
File path: server/src/main/java/org/apache/iotdb/db/utils/TestOnly.java
##
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+package org.apache.iotdb.db.utils;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Target({ElementType.METHOD, ElementType.CONSTRUCTOR})
+@Retention(RetentionPolicy.SOURCE)
+public @interface TestOnly {
Review comment:
Yes. Although some external packages already include this annotation, I do
not want to import them just for adding one annotation.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #429: [IOTDB-205]Support storage-group-level data ttl
jt2594838 commented on a change in pull request #429: [IOTDB-205]Support
storage-group-level data ttl
URL: https://github.com/apache/incubator-iotdb/pull/429#discussion_r337307360
##
File path: server/src/main/java/org/apache/iotdb/db/qp/logical/Operator.java
##
@@ -73,6 +73,7 @@ public String toString() {
GRANT_USER_PRIVILEGE, REVOKE_USER_PRIVILEGE, GRANT_USER_ROLE,
REVOKE_USER_ROLE, CREATE_ROLE,
DELETE_ROLE, GRANT_ROLE_PRIVILEGE, REVOKE_ROLE_PRIVILEGE, LIST_USER,
LIST_ROLE,
LIST_USER_PRIVILEGE, LIST_ROLE_PRIVILEGE, LIST_USER_ROLES, LIST_ROLE_USERS,
-GRANT_WATERMARK_EMBEDDING, REVOKE_WATERMARK_EMBEDDING,
DELETE_STORAGE_GROUP,
+GRANT_WATERMARK_EMBEDDING, REVOKE_WATERMARK_EMBEDDING,
+TTL, DELETE_STORAGE_GROUP
Review comment:
See the previous reply.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #429: [IOTDB-205]Support storage-group-level data ttl
jt2594838 commented on a change in pull request #429: [IOTDB-205]Support
storage-group-level data ttl
URL: https://github.com/apache/incubator-iotdb/pull/429#discussion_r337307205
##
File path:
server/src/main/java/org/apache/iotdb/db/metadata/MetadataOperationType.java
##
@@ -34,5 +34,6 @@ private MetadataOperationType(){
public static final String UNLINK_MNODE_FROM_PTREE = "7";
public static final String ADD_INDEX_TO_PATH = "8";
public static final String DELETE_INDEX_FROM_PATH = "9";
- public static final String DELETE_STORAGE_GROUP_FROM_MTREE = "10";
+ public static final String SET_TTL = "10";
+ public static final String DELETE_STORAGE_GROUP_FROM_MTREE = "11";
Review comment:
Because when I created the field SET_TTL, there is no such field
DELETE_STORAGE_GROUP_FROM_MTREE. And later I merged master which brought
DELETE_STORAGE_GROUP_FROM_MTREE.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying timeseries interface cannot make a query by the specified path prefix
qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying
timeseries interface cannot make a query by the specified path prefix
URL: https://github.com/apache/incubator-iotdb/pull/435#discussion_r337306993
##
File path: server/src/main/java/org/apache/iotdb/db/metadata/MTree.java
##
@@ -781,20 +777,56 @@ private void findStorageGroup(MNode node, String path,
HashSet res) {
*
* @return a list contains all distinct devices
*/
- Set getAllDevices() {
-return new HashSet<>(getNodesList(3));
+ Set getAllDevices() throws SQLException {
+List res = new ArrayList<>();
+MNode node;
+if ((node = getRoot()) != null) {
+ findDevices(node, "root", res);
+}
+return new LinkedHashSet<>(res);
+ }
+
+ private void findDevices(MNode node, String path, List res) {
+if (node == null) {
+ return;
+}
+if (node.isLeaf()) {
+ res.add(path);
+ return;
+}
+if (node.hasChildren()) {
+ for (MNode child : node.getChildren().values()) {
+if (child.isLeaf()) {
+ findDevices(child, path, res);
+} else {
+ findDevices(child, path + "." + child.toString(), res);
+}
+ }
+}
}
/**
* Get all nodes at the given level in current Metadata Tree.
*
* @return a list contains all nodes at the given level
*/
- List getNodesList(int nodeLevel) {
+ List getNodesList(String schemaPattern, int nodeLevel) throws
SQLException {
Review comment:
Is this function only for the count? If so, why not return an int?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying timeseries interface cannot make a query by the specified path prefix
qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying
timeseries interface cannot make a query by the specified path prefix
URL: https://github.com/apache/incubator-iotdb/pull/435#discussion_r337306655
##
File path: server/src/main/java/org/apache/iotdb/db/metadata/MTree.java
##
@@ -781,20 +777,56 @@ private void findStorageGroup(MNode node, String path,
HashSet res) {
*
* @return a list contains all distinct devices
*/
- Set getAllDevices() {
-return new HashSet<>(getNodesList(3));
+ Set getAllDevices() throws SQLException {
+List res = new ArrayList<>();
+MNode node;
+if ((node = getRoot()) != null) {
+ findDevices(node, "root", res);
+}
+return new LinkedHashSet<>(res);
+ }
+
+ private void findDevices(MNode node, String path, List res) {
Review comment:
This function is very confused, what's the relationship between node and
path?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying timeseries interface cannot make a query by the specified path prefix
qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying
timeseries interface cannot make a query by the specified path prefix
URL: https://github.com/apache/incubator-iotdb/pull/435#discussion_r337306522
##
File path: server/src/main/java/org/apache/iotdb/db/metadata/MTree.java
##
@@ -781,20 +777,56 @@ private void findStorageGroup(MNode node, String path,
HashSet res) {
*
* @return a list contains all distinct devices
*/
- Set getAllDevices() {
-return new HashSet<>(getNodesList(3));
+ Set getAllDevices() throws SQLException {
+List res = new ArrayList<>();
+MNode node;
+if ((node = getRoot()) != null) {
+ findDevices(node, "root", res);
+}
+return new LinkedHashSet<>(res);
+ }
+
+ private void findDevices(MNode node, String path, List res) {
+if (node == null) {
+ return;
+}
+if (node.isLeaf()) {
+ res.add(path);
+ return;
+}
+if (node.hasChildren()) {
+ for (MNode child : node.getChildren().values()) {
+if (child.isLeaf()) {
+ findDevices(child, path, res);
Review comment:
```suggestion
res.add(path);
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #429: [IOTDB-205]Support storage-group-level data ttl
jt2594838 commented on a change in pull request #429: [IOTDB-205]Support
storage-group-level data ttl
URL: https://github.com/apache/incubator-iotdb/pull/429#discussion_r337306179
##
File path:
server/src/main/java/org/apache/iotdb/db/engine/merge/manage/MergeResource.java
##
@@ -68,9 +68,21 @@
private boolean cacheDeviceMeta = false;
public MergeResource(List seqFiles, List
unseqFiles) {
-this.seqFiles =
seqFiles.stream().filter(TsFileResource::isClosed).collect(Collectors.toList());
+this.seqFiles = seqFiles.stream().filter(res -> res.isClosed() &&
!res.isDeleted())
+.collect(Collectors.toList());
this.unseqFiles =
-
unseqFiles.stream().filter(TsFileResource::isClosed).collect(Collectors.toList());
+unseqFiles.stream().filter(res -> res.isClosed() && !res.isDeleted())
+.collect(Collectors.toList());
+ }
+
+ public MergeResource(List seqFiles, List
unseqFiles,
+ long timeBound) {
+this.seqFiles =
+seqFiles.stream().filter(res -> res.isClosed() && !res.isDeleted()
+&& res.stillLives(timeBound)).collect(Collectors.toList());
+this.unseqFiles =
+unseqFiles.stream().filter(res -> res.isClosed() && !res.isDeleted()
+&& res.stillLives(timeBound)).collect(Collectors.toList());
Review comment:
Yes, but considering the frequency of merge (at least every tens of
minutes), such improvement is negligible.
Still, I replace the lambda with a method reference, please have a look.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying timeseries interface cannot make a query by the specified path prefix
qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying
timeseries interface cannot make a query by the specified path prefix
URL: https://github.com/apache/incubator-iotdb/pull/435#discussion_r337306085
##
File path: server/src/main/java/org/apache/iotdb/db/metadata/MTree.java
##
@@ -781,20 +777,56 @@ private void findStorageGroup(MNode node, String path,
HashSet res) {
*
* @return a list contains all distinct devices
*/
- Set getAllDevices() {
-return new HashSet<>(getNodesList(3));
+ Set getAllDevices() throws SQLException {
+List res = new ArrayList<>();
+MNode node;
+if ((node = getRoot()) != null) {
+ findDevices(node, "root", res);
+}
+return new LinkedHashSet<>(res);
+ }
+
+ private void findDevices(MNode node, String path, List res) {
+if (node == null) {
+ return;
+}
+if (node.isLeaf()) {
+ res.add(path);
+ return;
+}
+if (node.hasChildren()) {
Review comment:
You already check the node is null or is a leaf. Here, the node should have
a child. This if statement is not needed.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying timeseries interface cannot make a query by the specified path prefix
qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying
timeseries interface cannot make a query by the specified path prefix
URL: https://github.com/apache/incubator-iotdb/pull/435#discussion_r337305728
##
File path: server/src/main/java/org/apache/iotdb/db/metadata/MTree.java
##
@@ -781,20 +777,56 @@ private void findStorageGroup(MNode node, String path,
HashSet res) {
*
* @return a list contains all distinct devices
*/
- Set getAllDevices() {
-return new HashSet<>(getNodesList(3));
+ Set getAllDevices() throws SQLException {
+List res = new ArrayList<>();
+MNode node;
+if ((node = getRoot()) != null) {
+ findDevices(node, "root", res);
Review comment:
```suggestion
findDevices(node, SQLConstant.ROOT, res);
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] fanhualta opened a new pull request #465: [IOTDB-261]Check path validity in session
fanhualta opened a new pull request #465: [IOTDB-261]Check path validity in session URL: https://github.com/apache/incubator-iotdb/pull/465 When using Session to set storage group or create time series, the path needs to be checked in order to reject this creation if it contains special characters. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-iotdb] qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying timeseries interface cannot make a query by the specified path prefix
qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying
timeseries interface cannot make a query by the specified path prefix
URL: https://github.com/apache/incubator-iotdb/pull/435#discussion_r337304254
##
File path: server/src/main/java/org/apache/iotdb/db/metadata/MTree.java
##
@@ -781,20 +777,56 @@ private void findStorageGroup(MNode node, String path,
HashSet res) {
*
* @return a list contains all distinct devices
*/
- Set getAllDevices() {
-return new HashSet<>(getNodesList(3));
+ Set getAllDevices() throws SQLException {
+List res = new ArrayList<>();
Review comment:
```suggestion
List devices = new ArrayList<>();
```
And, why not define a set directly
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #429: [IOTDB-205]Support storage-group-level data ttl
jt2594838 commented on a change in pull request #429: [IOTDB-205]Support
storage-group-level data ttl
URL: https://github.com/apache/incubator-iotdb/pull/429#discussion_r337304790
##
File path:
server/src/main/java/org/apache/iotdb/db/engine/storagegroup/StorageGroupProcessor.java
##
@@ -580,6 +610,65 @@ public void syncDeleteDataFiles() {
}
}
+ /**
+ * Iterate each TsFile and try to lock and remove those out of TTL.
+ */
+ public synchronized void checkFilesTTL() {
+if (dataTTL == Long.MAX_VALUE) {
+ logger.debug("{}: TTL not set, ignore the check", storageGroupName);
+ return;
+}
+long timeBound = System.currentTimeMillis() - dataTTL;
+if (logger.isDebugEnabled()) {
+ logger.debug("{}: TTL removing files before {}", storageGroupName, new
Date(timeBound));
+}
+// copy to avoid concurrent modification of deletion
+List seqFiles = new ArrayList<>(sequenceFileList);
+List unseqFiles = new ArrayList<>(unSequenceFileList);
+
+for (TsFileResource tsFileResource : seqFiles) {
+ checkFileTTL(tsFileResource, timeBound, true);
+}
+for (TsFileResource tsFileResource : unseqFiles) {
+ checkFileTTL(tsFileResource, timeBound, false);
+}
Review comment:
Unfortunately, the Iterator of ArrayList does not provide concurrent safety,
either.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying timeseries interface cannot make a query by the specified path prefix
qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying
timeseries interface cannot make a query by the specified path prefix
URL: https://github.com/apache/incubator-iotdb/pull/435#discussion_r337304603
##
File path: server/src/main/java/org/apache/iotdb/db/metadata/MTree.java
##
@@ -781,20 +777,56 @@ private void findStorageGroup(MNode node, String path,
HashSet res) {
*
* @return a list contains all distinct devices
*/
- Set getAllDevices() {
-return new HashSet<>(getNodesList(3));
+ Set getAllDevices() throws SQLException {
Review comment:
```suggestion
Set getAllDevices() {
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying timeseries interface cannot make a query by the specified path prefix
qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying
timeseries interface cannot make a query by the specified path prefix
URL: https://github.com/apache/incubator-iotdb/pull/435#discussion_r337304254
##
File path: server/src/main/java/org/apache/iotdb/db/metadata/MTree.java
##
@@ -781,20 +777,56 @@ private void findStorageGroup(MNode node, String path,
HashSet res) {
*
* @return a list contains all distinct devices
*/
- Set getAllDevices() {
-return new HashSet<>(getNodesList(3));
+ Set getAllDevices() throws SQLException {
+List res = new ArrayList<>();
Review comment:
```suggestion
List devices = new ArrayList<>();
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying timeseries interface cannot make a query by the specified path prefix
qiaojialin commented on a change in pull request #435: [IOTDB-174] Fix querying
timeseries interface cannot make a query by the specified path prefix
URL: https://github.com/apache/incubator-iotdb/pull/435#discussion_r337304127
##
File path: server/src/main/java/org/apache/iotdb/db/metadata/MManager.java
##
@@ -830,11 +831,11 @@ public Metadata getMetadata() throws PathErrorException {
*
* @return A List instance which stores all node at given level
*/
- public List getNodesList(int nodeLevel) {
+ public List getNodesList(String schemaPattern, int nodeLevel) throws
SQLException {
Review comment:
```suggestion
public List getNodesList(String prefixPath, int nodeLevel) throws
SQLException {
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] JackieTien97 commented on a change in pull request #429: [IOTDB-205]Support storage-group-level data ttl
JackieTien97 commented on a change in pull request #429: [IOTDB-205]Support
storage-group-level data ttl
URL: https://github.com/apache/incubator-iotdb/pull/429#discussion_r337062725
##
File path: server/src/main/java/org/apache/iotdb/db/utils/TestOnly.java
##
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+package org.apache.iotdb.db.utils;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Target({ElementType.METHOD, ElementType.CONSTRUCTOR})
+@Retention(RetentionPolicy.SOURCE)
+public @interface TestOnly {
Review comment:
Does this mean that any function with this annotation should only be used in
UT and IT?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] JackieTien97 commented on a change in pull request #429: [IOTDB-205]Support storage-group-level data ttl
JackieTien97 commented on a change in pull request #429: [IOTDB-205]Support
storage-group-level data ttl
URL: https://github.com/apache/incubator-iotdb/pull/429#discussion_r337052801
##
File path: server/src/main/java/org/apache/iotdb/db/qp/logical/Operator.java
##
@@ -73,6 +73,7 @@ public String toString() {
GRANT_USER_PRIVILEGE, REVOKE_USER_PRIVILEGE, GRANT_USER_ROLE,
REVOKE_USER_ROLE, CREATE_ROLE,
DELETE_ROLE, GRANT_ROLE_PRIVILEGE, REVOKE_ROLE_PRIVILEGE, LIST_USER,
LIST_ROLE,
LIST_USER_PRIVILEGE, LIST_ROLE_PRIVILEGE, LIST_USER_ROLES, LIST_ROLE_USERS,
-GRANT_WATERMARK_EMBEDDING, REVOKE_WATERMARK_EMBEDDING,
DELETE_STORAGE_GROUP,
+GRANT_WATERMARK_EMBEDDING, REVOKE_WATERMARK_EMBEDDING,
+TTL, DELETE_STORAGE_GROUP
Review comment:
Why put TTL before DELETE_STORAGE_GROUP instead of adding it to the end.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] LeiRui commented on issue #455: [IOTDB-251]improve TSQueryDataSet structure in RPC
LeiRui commented on issue #455: [IOTDB-251]improve TSQueryDataSet structure in RPC URL: https://github.com/apache/incubator-iotdb/pull/455#issuecomment-544514616 > How about this: > Deprecate the List of RowRecord and instead use a RowBatch, which avoids too many objects such as Fields and RowRecord itself. Hi, thank you, I have recorded your thought on JIRA https://github.com/apache/incubator-iotdb/pull/455 for future work. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-iotdb] HTHou commented on a change in pull request #464: Modified Decoder and SequenceReader to support old version of TsFile
HTHou commented on a change in pull request #464: Modified Decoder and
SequenceReader to support old version of TsFile
URL: https://github.com/apache/incubator-iotdb/pull/464#discussion_r337007967
##
File path:
tsfile/src/main/java/org/apache/iotdb/tsfile/file/metadata/TsFileMetaData.java
##
@@ -150,9 +150,11 @@ public static TsFileMetaData deserializeFrom(ByteBuffer
buffer) throws IOExcepti
if (ReadWriteIOUtils.readIsNull(buffer)) {
fileMetaData.createdBy = ReadWriteIOUtils.readString(buffer);
-}
-fileMetaData.totalChunkNum = ReadWriteIOUtils.readInt(buffer);
-fileMetaData.invalidChunkNum = ReadWriteIOUtils.readInt(buffer);
+}
+// if using v0.8.0 TsFile, use 0 to represent missing fields
+fileMetaData.totalChunkNum = buffer == null ? 0 :
ReadWriteIOUtils.readInt(buffer);
+fileMetaData.invalidChunkNum = buffer == null ? 0 :
ReadWriteIOUtils.readInt(buffer);
Review comment:
> Could this buffer really be null?
Did you mean I should change `buffer == null`to
`!ReadWriteIOUtils.readIsNull(buffer)` ?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] LeiRui commented on a change in pull request #455: [IOTDB-251]improve TSQueryDataSet structure in RPC
LeiRui commented on a change in pull request #455: [IOTDB-251]improve
TSQueryDataSet structure in RPC
URL: https://github.com/apache/incubator-iotdb/pull/455#discussion_r337007825
##
File path:
server/src/main/java/org/apache/iotdb/db/utils/QueryDataSetUtils.java
##
@@ -55,66 +55,86 @@ public static TSQueryDataSet
convertQueryDataSetByFetchSize(QueryDataSet queryDa
public static TSQueryDataSet convertQueryDataSetByFetchSize(QueryDataSet
queryDataSet,
int fetchSize, WatermarkEncoder watermarkEncoder) throws IOException {
+List dataTypes = queryDataSet.getDataTypes();
+int columnNum = dataTypes.size();
TSQueryDataSet tsQueryDataSet = new TSQueryDataSet();
-tsQueryDataSet.setRecords(new ArrayList<>());
+int columnNumWithTime = columnNum + 1;
+DataOutputStream[] dataOutputStreams = new
DataOutputStream[columnNumWithTime];
+ByteArrayOutputStream[] byteArrayOutputStreams = new
ByteArrayOutputStream[columnNumWithTime];
+for (int i = 0; i < columnNumWithTime; i++) {
+ byteArrayOutputStreams[i] = new ByteArrayOutputStream();
+ dataOutputStreams[i] = new DataOutputStream(byteArrayOutputStreams[i]);
+}
+
+int rowCount = 0;
+int valueOccupation = 0;
for (int i = 0; i < fetchSize; i++) {
if (queryDataSet.hasNext()) {
+rowCount++;
RowRecord rowRecord = queryDataSet.next();
if (watermarkEncoder != null) {
rowRecord = watermarkEncoder.encodeRecord(rowRecord);
}
-tsQueryDataSet.getRecords().add(convertToTSRecord(rowRecord));
+// use columnOutput to write byte array
+dataOutputStreams[0].writeLong(rowRecord.getTimestamp());
+List fields = rowRecord.getFields();
+for (int k = 0; k < fields.size(); k++) {
+ Field field = fields.get(k);
+ DataOutputStream dataOutputStream = dataOutputStreams[k + 1]; // DO
NOT FORGET +1
+ if (field.getDataType() == null) {
+dataOutputStream.writeBoolean(true); // is_empty true
+ } else {
+dataOutputStream.writeBoolean(false); // is_empty false
Review comment:
Hi, yes, this is scheduled as a possible future improvement as for the inner
structure of TSQueryDataSet's value buffer. And more experiments are needed.
But for now, the core aim of this pr, which is to remove record-wise
structure in TSQueryDataSet, is hit.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] HTHou commented on a change in pull request #464: Modified Decoder and SequenceReader to support old version of TsFile
HTHou commented on a change in pull request #464: Modified Decoder and
SequenceReader to support old version of TsFile
URL: https://github.com/apache/incubator-iotdb/pull/464#discussion_r337001576
##
File path:
tsfile/src/main/java/org/apache/iotdb/tsfile/read/TsFileSequenceReader.java
##
@@ -346,6 +341,12 @@ private ChunkHeader readChunkHeader(long position,
boolean markerRead) throws IO
* @return the pages of this chunk
*/
public ByteBuffer readChunk(ChunkHeader header) throws IOException {
+if (this.readVersionNumber().startsWith("v")) {
+ config.setEndian("LITTLE_ENDIAN");
+}
+else {
+ config.setEndian("BIG_ENDIAN");
+}
Review comment:
> I think this will cause a mess if two files of different endian are being
read concurrently.
Yes, it may cause a mess, but is there any other idea about how and where to
determine the endian type?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] samperson1997 commented on a change in pull request #435: [IOTDB-174] Fix querying timeseries interface cannot make a query by the specified path prefix
samperson1997 commented on a change in pull request #435: [IOTDB-174] Fix
querying timeseries interface cannot make a query by the specified path prefix
URL: https://github.com/apache/incubator-iotdb/pull/435#discussion_r337003272
##
File path: server/src/main/java/org/apache/iotdb/db/metadata/MTree.java
##
@@ -782,20 +783,28 @@ private void findStorageGroup(MNode node, String path,
HashSet res) {
*
* @return a list contains all distinct devices
*/
- Set getAllDevices() {
-return new HashSet<>(getNodesList(3));
+ Set getAllDevices() throws SQLException {
+return new HashSet<>(getNodesList("root",2));
Review comment:
Hi, terribly sorry for the late reply, and I’m responsible for this issue.
Do you think it is necessary to add a new param `String schemaPattern` for
method `Set getAllDevices()` as well?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] HTHou commented on a change in pull request #464: Modified Decoder and SequenceReader to support old version of TsFile
HTHou commented on a change in pull request #464: Modified Decoder and
SequenceReader to support old version of TsFile
URL: https://github.com/apache/incubator-iotdb/pull/464#discussion_r337001576
##
File path:
tsfile/src/main/java/org/apache/iotdb/tsfile/read/TsFileSequenceReader.java
##
@@ -346,6 +341,12 @@ private ChunkHeader readChunkHeader(long position,
boolean markerRead) throws IO
* @return the pages of this chunk
*/
public ByteBuffer readChunk(ChunkHeader header) throws IOException {
+if (this.readVersionNumber().startsWith("v")) {
+ config.setEndian("LITTLE_ENDIAN");
+}
+else {
+ config.setEndian("BIG_ENDIAN");
+}
Review comment:
> I think this will cause a mess if two files of different endian are being
read concurrently.
Yes, it may cause a mess, but I have no other idea about how and where to
determine the endian type.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #464: Modified Decoder and SequenceReader to support old version of TsFile
jt2594838 commented on a change in pull request #464: Modified Decoder and
SequenceReader to support old version of TsFile
URL: https://github.com/apache/incubator-iotdb/pull/464#discussion_r336984734
##
File path:
tsfile/src/main/java/org/apache/iotdb/tsfile/read/TsFileSequenceReader.java
##
@@ -346,6 +341,12 @@ private ChunkHeader readChunkHeader(long position,
boolean markerRead) throws IO
* @return the pages of this chunk
*/
public ByteBuffer readChunk(ChunkHeader header) throws IOException {
+if (this.readVersionNumber().startsWith("v")) {
+ config.setEndian("LITTLE_ENDIAN");
+}
+else {
+ config.setEndian("BIG_ENDIAN");
+}
Review comment:
I think this will cause a mess if two files of different endian are being
read concurrently.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #464: Modified Decoder and SequenceReader to support old version of TsFile
jt2594838 commented on a change in pull request #464: Modified Decoder and
SequenceReader to support old version of TsFile
URL: https://github.com/apache/incubator-iotdb/pull/464#discussion_r336980811
##
File path:
tsfile/src/main/java/org/apache/iotdb/tsfile/encoding/decoder/PlainDecoder.java
##
@@ -54,26 +55,55 @@ public boolean readBoolean(ByteBuffer buffer) {
@Override
public short readShort(ByteBuffer buffer) {
+if (this.endianType == EndianType.LITTLE_ENDIAN) {
+ int ch1 = ReadWriteIOUtils.read(buffer);
+ int ch2 = ReadWriteIOUtils.read(buffer);
+ return (short) (ch1 + (ch2 << 8));
+}
return buffer.getShort();
Review comment:
A better way would be setting the endian of the buffer using
`byteBuffer.order()` when the buffer is created, which can avoid many redundant
checks.
Notice: ByteBuffers get from methods like `byteBuffer.slice()` do not
inherit the byte order, so you have to set byte order for those ByteBuffers.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #464: Modified Decoder and SequenceReader to support old version of TsFile
jt2594838 commented on a change in pull request #464: Modified Decoder and
SequenceReader to support old version of TsFile
URL: https://github.com/apache/incubator-iotdb/pull/464#discussion_r336983539
##
File path:
tsfile/src/main/java/org/apache/iotdb/tsfile/file/metadata/TsFileMetaData.java
##
@@ -150,9 +150,11 @@ public static TsFileMetaData deserializeFrom(ByteBuffer
buffer) throws IOExcepti
if (ReadWriteIOUtils.readIsNull(buffer)) {
fileMetaData.createdBy = ReadWriteIOUtils.readString(buffer);
-}
-fileMetaData.totalChunkNum = ReadWriteIOUtils.readInt(buffer);
-fileMetaData.invalidChunkNum = ReadWriteIOUtils.readInt(buffer);
+}
+// if using v0.8.0 TsFile, use 0 to represent missing fields
+fileMetaData.totalChunkNum = buffer == null ? 0 :
ReadWriteIOUtils.readInt(buffer);
+fileMetaData.invalidChunkNum = buffer == null ? 0 :
ReadWriteIOUtils.readInt(buffer);
Review comment:
Could this buffer really be null?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #464: Modified Decoder and SequenceReader to support old version of TsFile
jt2594838 commented on a change in pull request #464: Modified Decoder and
SequenceReader to support old version of TsFile
URL: https://github.com/apache/incubator-iotdb/pull/464#discussion_r336981867
##
File path:
tsfile/src/main/java/org/apache/iotdb/tsfile/file/metadata/TsDigest.java
##
@@ -65,18 +67,49 @@ public static int serializeNullTo(ByteBuffer buffer) {
*/
public static TsDigest deserializeFrom(InputStream inputStream) throws
IOException {
TsDigest digest = new TsDigest();
-
int size = ReadWriteIOUtils.readInt(inputStream);
digest.validSizeOfArray = size;
digest.serializedSize = Integer.BYTES;
if (size > 0) {
digest.statistics = new ByteBuffer[StatisticType.getTotalTypeNum()];
ByteBuffer value;
- for (int i = 0; i < size; i++) {
-short n = ReadWriteIOUtils.readShort(inputStream);
-value =
ReadWriteIOUtils.readByteBufferWithSelfDescriptionLength(inputStream);
-digest.statistics[n] = value;
-digest.serializedSize += Short.BYTES + Integer.BYTES +
value.remaining();
+ // check if it's old version of TsFile
+ String key = "";
+ if
(TSFileDescriptor.getInstance().getConfig().getEndian().equals("LITTLE_ENDIAN"))
{
+ for (int i = 0; i < size; i++) {
+ key = ReadWriteIOUtils.readString(inputStream);
+ value =
ReadWriteIOUtils.readByteBufferWithSelfDescriptionLength(inputStream);
+ short n;
+ switch (key) {
+case "min_value":
+ n = 0;
+ break;
+ case "max_value":
+ n = 1;
+ break;
+ case "first":
+ n = 2;
+ break;
+ case "last":
+ n = 3;
+ break;
+ case "sum":
+ n = 4;
+ break;
+ default:
+ n = -1;
+ }
+ digest.statistics[n] = value;
+ digest.serializedSize += Short.BYTES + Integer.BYTES +
value.remaining();
+ }
+ }
+ else {
+ for (int i = 0; i < size; i++) {
+ short n = ReadWriteIOUtils.readShort(inputStream);
+ value =
ReadWriteIOUtils.readByteBufferWithSelfDescriptionLength(inputStream);
+ digest.statistics[n] = value;
+ digest.serializedSize += Short.BYTES + Integer.BYTES +
value.remaining();
+ }
Review comment:
Please reformat the above codes.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #462: [IOTDB-253]time expression
jt2594838 commented on a change in pull request #462: [IOTDB-253]time expression
URL: https://github.com/apache/incubator-iotdb/pull/462#discussion_r336973867
##
File path:
server/src/main/java/org/apache/iotdb/db/qp/constant/DatetimeUtils.java
##
@@ -475,6 +475,66 @@ public static long convertDatetimeStrToLong(String str,
ZoneOffset offset, int d
return getInstantWithPrecision(str, timestampPrecision);
}
+ /**
+ * convert duration string to millisecond, microsecond or nanosecond.
+ */
+ public static long convertDurationStrToLong(long value, String unit, String
timestampPrecision) {
+
+long res = value;
+switch (unit) {
+ case "y":
+res *= 365 * 86400_000;
+break;
+ case "mo":
+res *= 30 * 86400_000;
+break;
+ case "w":
+res *= 7 * 86400_000;
+break;
+ case "d":
+res *= 86400_000;
+break;
+ case "h":
+res *= 3600_000;
+break;
+ case "m":
+res *= 60_000;
+break;
+ case "s":
+res *= 1_000;
+break;
+ default:
Review comment:
`365 * 86400_000` and `30 * 86400_000` overflow int, please use `365 *
86400_000L`.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #462: [IOTDB-253]time expression
jt2594838 commented on a change in pull request #462: [IOTDB-253]time expression
URL: https://github.com/apache/incubator-iotdb/pull/462#discussion_r336970873
##
File path: docs/Documentation-CHN/UserGuide/2-Concept Key Concepts and
Terminology/1-Key Concepts and Terminology.md
##
@@ -166,6 +170,42 @@ IoTDB在显示时间戳时可以支持LONG类型以及DATETIME-DISPLAY类型，
+* 相对时间戳
+
+ 相对时间是指与服务器时间```now()```和```DATETIME```类型时间相差一定时间间隔的时间。
+ 形式化定义为：
+ ```
+ Duration = (Digit+ ('Y'|'MO'|'W'|'D'|'H'|'M'|'S'|'MS'|'US'|'NS'))+
+ RelativeTime = (now() | DATETIME) ((+|-) Duration)+
Review comment:
It seems durations like "5Y6Y", "7D8D" will also be accepted. How do you
handle that?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #455: [IOTDB-251]improve TSQueryDataSet structure in RPC
jt2594838 commented on a change in pull request #455: [IOTDB-251]improve
TSQueryDataSet structure in RPC
URL: https://github.com/apache/incubator-iotdb/pull/455#discussion_r336965978
##
File path:
server/src/main/java/org/apache/iotdb/db/utils/QueryDataSetUtils.java
##
@@ -55,66 +55,86 @@ public static TSQueryDataSet
convertQueryDataSetByFetchSize(QueryDataSet queryDa
public static TSQueryDataSet convertQueryDataSetByFetchSize(QueryDataSet
queryDataSet,
int fetchSize, WatermarkEncoder watermarkEncoder) throws IOException {
+List dataTypes = queryDataSet.getDataTypes();
+int columnNum = dataTypes.size();
TSQueryDataSet tsQueryDataSet = new TSQueryDataSet();
-tsQueryDataSet.setRecords(new ArrayList<>());
+int columnNumWithTime = columnNum + 1;
+DataOutputStream[] dataOutputStreams = new
DataOutputStream[columnNumWithTime];
+ByteArrayOutputStream[] byteArrayOutputStreams = new
ByteArrayOutputStream[columnNumWithTime];
+for (int i = 0; i < columnNumWithTime; i++) {
+ byteArrayOutputStreams[i] = new ByteArrayOutputStream();
+ dataOutputStreams[i] = new DataOutputStream(byteArrayOutputStreams[i]);
+}
+
+int rowCount = 0;
+int valueOccupation = 0;
for (int i = 0; i < fetchSize; i++) {
if (queryDataSet.hasNext()) {
+rowCount++;
RowRecord rowRecord = queryDataSet.next();
if (watermarkEncoder != null) {
rowRecord = watermarkEncoder.encodeRecord(rowRecord);
}
-tsQueryDataSet.getRecords().add(convertToTSRecord(rowRecord));
+// use columnOutput to write byte array
+dataOutputStreams[0].writeLong(rowRecord.getTimestamp());
+List fields = rowRecord.getFields();
+for (int k = 0; k < fields.size(); k++) {
+ Field field = fields.get(k);
+ DataOutputStream dataOutputStream = dataOutputStreams[k + 1]; // DO
NOT FORGET +1
+ if (field.getDataType() == null) {
+dataOutputStream.writeBoolean(true); // is_empty true
+ } else {
+dataOutputStream.writeBoolean(false); // is_empty false
Review comment:
Optional: replace the booleans with a bit map which induce less overhead.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] JackieTien97 commented on a change in pull request #429: [IOTDB-205]Support storage-group-level data ttl
JackieTien97 commented on a change in pull request #429: [IOTDB-205]Support
storage-group-level data ttl
URL: https://github.com/apache/incubator-iotdb/pull/429#discussion_r336945363
##
File path:
server/src/main/java/org/apache/iotdb/db/metadata/MetadataOperationType.java
##
@@ -34,5 +34,6 @@ private MetadataOperationType(){
public static final String UNLINK_MNODE_FROM_PTREE = "7";
public static final String ADD_INDEX_TO_PATH = "8";
public static final String DELETE_INDEX_FROM_PATH = "9";
- public static final String DELETE_STORAGE_GROUP_FROM_MTREE = "10";
+ public static final String SET_TTL = "10";
+ public static final String DELETE_STORAGE_GROUP_FROM_MTREE = "11";
Review comment:
Why not directly add a field SET_TTL = "11"?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 merged pull request #457: Update doc about spark package change
jt2594838 merged pull request #457: Update doc about spark package change URL: https://github.com/apache/incubator-iotdb/pull/457 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-iotdb] HTHou commented on issue #450: modify documents
HTHou commented on issue #450: modify documents URL: https://github.com/apache/incubator-iotdb/pull/450#issuecomment-544405816 > Please resolve the conflicts. Thank you so much for all your suggestions and comments. I'm going to working on the conflicts now. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-iotdb] JackieTien97 commented on a change in pull request #429: [IOTDB-205]Support storage-group-level data ttl
JackieTien97 commented on a change in pull request #429: [IOTDB-205]Support
storage-group-level data ttl
URL: https://github.com/apache/incubator-iotdb/pull/429#discussion_r336877055
##
File path:
server/src/main/java/org/apache/iotdb/db/engine/storagegroup/TsFileProcessor.java
##
@@ -576,6 +579,8 @@ public String getStorageGroupName() {
QueryUtils.modifyChunkMetaData(chunkMetaDataList,
modifications);
+ chunkMetaDataList.removeIf(chunkMetaData -> chunkMetaData.getEndTime() <
timeLowerBound);
Review comment:
Still the same question. The variable `timeLowerBound` will cause class
construction every time the function is invoked.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 merged pull request #458: [IOTDB-165][Documents][TsFile] Fix a example and update tsfile documents.
jt2594838 merged pull request #458: [IOTDB-165][Documents][TsFile] Fix a example and update tsfile documents. URL: https://github.com/apache/incubator-iotdb/pull/458 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-iotdb] JackieTien97 commented on a change in pull request #429: [IOTDB-205]Support storage-group-level data ttl
JackieTien97 commented on a change in pull request #429: [IOTDB-205]Support
storage-group-level data ttl
URL: https://github.com/apache/incubator-iotdb/pull/429#discussion_r336874974
##
File path:
server/src/main/java/org/apache/iotdb/db/engine/merge/manage/MergeResource.java
##
@@ -68,9 +68,21 @@
private boolean cacheDeviceMeta = false;
public MergeResource(List seqFiles, List
unseqFiles) {
-this.seqFiles =
seqFiles.stream().filter(TsFileResource::isClosed).collect(Collectors.toList());
+this.seqFiles = seqFiles.stream().filter(res -> res.isClosed() &&
!res.isDeleted())
+.collect(Collectors.toList());
this.unseqFiles =
-
unseqFiles.stream().filter(TsFileResource::isClosed).collect(Collectors.toList());
+unseqFiles.stream().filter(res -> res.isClosed() && !res.isDeleted())
+.collect(Collectors.toList());
+ }
+
+ public MergeResource(List seqFiles, List
unseqFiles,
+ long timeBound) {
+this.seqFiles =
+seqFiles.stream().filter(res -> res.isClosed() && !res.isDeleted()
+&& res.stillLives(timeBound)).collect(Collectors.toList());
+this.unseqFiles =
+unseqFiles.stream().filter(res -> res.isClosed() && !res.isDeleted()
+&& res.stillLives(timeBound)).collect(Collectors.toList());
Review comment:
Every time the function `MergeResource` is invoked, two new class will be
created because the `timeBound` in lambda express may be different among
different invocations.
It's better to write a class to implement the interface Predicate and
maintain a field named `timeBound`. And set the field to different value in the
function `MergeResource`.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on issue #450: modify documents
jt2594838 commented on issue #450: modify documents URL: https://github.com/apache/incubator-iotdb/pull/450#issuecomment-544387184 Please resolve the conflicts. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-iotdb] jt2594838 commented on a change in pull request #450: modify documents
jt2594838 commented on a change in pull request #450: modify documents URL: https://github.com/apache/incubator-iotdb/pull/450#discussion_r336869424 ## File path: docs/Documentation/UserGuide/1-Overview/4-Features.md ## @@ -43,9 +43,9 @@ IoTDB supports millions of low-power devices' strong connection data access, hig IoTDB supports time alignment for timeseries data accross devices and sensors, computation in timeseries field (frequency domain transformation) and rich aggregation function support in time dimension. -* Easy to get start. +* Easy to get startd. Review comment: startd -> started 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-iotdb] JackieTien97 commented on a change in pull request #429: [IOTDB-205]Support storage-group-level data ttl
JackieTien97 commented on a change in pull request #429: [IOTDB-205]Support
storage-group-level data ttl
URL: https://github.com/apache/incubator-iotdb/pull/429#discussion_r336868641
##
File path:
server/src/main/java/org/apache/iotdb/db/engine/storagegroup/StorageGroupProcessor.java
##
@@ -580,6 +610,65 @@ public void syncDeleteDataFiles() {
}
}
+ /**
+ * Iterate each TsFile and try to lock and remove those out of TTL.
+ */
+ public synchronized void checkFilesTTL() {
+if (dataTTL == Long.MAX_VALUE) {
+ logger.debug("{}: TTL not set, ignore the check", storageGroupName);
+ return;
+}
+long timeBound = System.currentTimeMillis() - dataTTL;
+if (logger.isDebugEnabled()) {
+ logger.debug("{}: TTL removing files before {}", storageGroupName, new
Date(timeBound));
+}
+// copy to avoid concurrent modification of deletion
+List seqFiles = new ArrayList<>(sequenceFileList);
+List unseqFiles = new ArrayList<>(unSequenceFileList);
+
+for (TsFileResource tsFileResource : seqFiles) {
+ checkFileTTL(tsFileResource, timeBound, true);
+}
+for (TsFileResource tsFileResource : unseqFiles) {
+ checkFileTTL(tsFileResource, timeBound, false);
+}
Review comment:
Why not use Iterator.remove() to avoid concurrent modification of deletion?
It can also save the extra list copy time and space.
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:
us...@infra.apache.org
With regards,
Apache Git Services
