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/07/21 02:12:22 UTC

[GitHub] [iotdb] yyt86 opened a new pull request #3606: delete TimeIndexLevel class and related methods in other classes

yyt86 opened a new pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606


   


-- 
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] coveralls edited a comment on pull request #3606: [IOTDB-1372] delete TimeIndexLevel class and related methods in other classes

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#issuecomment-883859342


   
   [![Coverage Status](https://coveralls.io/builds/41611735/badge)](https://coveralls.io/builds/41611735)
   
   Coverage decreased (-0.08%) to 68.083% when pulling **d77555c477f9e0388393d194c87fdea221c38f26 on yyt86:remove_device_set** into **79e50d651a2b488ac211009947b2a01c5244c37b on apache:master**.
   


-- 
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] jixuan1989 commented on pull request #3606: [IOTDB-1372] delete TimeIndexLevel class and related methods in other classes

Posted by GitBox <gi...@apache.org>.
jixuan1989 commented on pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#issuecomment-885982984


   The motivation?


-- 
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] jixuan1989 commented on pull request #3606: [IOTDB-1372] delete TimeIndexLevel class and related methods in other classes

Posted by GitBox <gi...@apache.org>.
jixuan1989 commented on pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#issuecomment-885982952


   What is this PR for?


-- 
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] yyt86 commented on a change in pull request #3606: [IOTDB-1372] delete devices field in FileTimeIndex

Posted by GitBox <gi...@apache.org>.
yyt86 commented on a change in pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#discussion_r683740184



##########
File path: server/src/main/java/org/apache/iotdb/db/engine/storagegroup/timeindex/FileTimeIndex.java
##########
@@ -114,8 +81,14 @@ public void close() {
   }
 
   @Override
-  public Set<String> getDevices() {
-    return devices;
+  public Set<String> getDevices(String tsFilePath) {
+    try {
+      TsFileSequenceReader fileReader = FileReaderManager.getInstance().get(tsFilePath, true);
+      return new HashSet<>(fileReader.getAllDevices());
+    } catch (IOException e) {
+      logger.error("Can't read file {} from disk ", tsFilePath, e);

Review comment:
       I will create another PR to throw IOException for the method and all method callers.




-- 
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] wangchao316 commented on a change in pull request #3606: delete TimeIndexLevel class and related methods in other classes

Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#discussion_r673712803



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/RemoteTsFileResource.java
##########
@@ -47,7 +47,7 @@
 
   public RemoteTsFileResource() {
     setClosed(true);
-    this.timeIndex = IoTDBDescriptor.getInstance().getConfig().getTimeIndexLevel().getTimeIndex();
+    this.timeIndex = new DeviceTimeIndex();

Review comment:
       hi , Thanks your contribution.
   I have a minor question .  why does delete TimeIndexlevel ? could you please write some reason? or 
   first , create a jira issue or github issue for desc your purpose.
   and then, raise pr link this issue. 
   this way for understands.




-- 
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] coveralls commented on pull request #3606: delete TimeIndexLevel class and related methods in other classes

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#issuecomment-883859342


   
   [![Coverage Status](https://coveralls.io/builds/41527314/badge)](https://coveralls.io/builds/41527314)
   
   Coverage decreased (-0.03%) to 68.136% when pulling **fc1434140db715e939b4c6d8b4f437ace07cd463 on yyt86:remove_device_set** into **79e50d651a2b488ac211009947b2a01c5244c37b on apache:master**.
   


-- 
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] coveralls edited a comment on pull request #3606: [IOTDB-1372] delete devices field in FileTimeIndex

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#issuecomment-883859342


   
   [![Coverage Status](https://coveralls.io/builds/41966120/badge)](https://coveralls.io/builds/41966120)
   
   Coverage decreased (-0.02%) to 68.255% when pulling **15070832475cae8b70913fe01561b471bfac5b21 on yyt86:remove_device_set** into **785e316b3670ed25e0d966c00d09bf2aad4097ab on apache:master**.
   


-- 
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] jixuan1989 edited a comment on pull request #3606: [IOTDB-1372] delete TimeIndexLevel class and related methods in other classes

Posted by GitBox <gi...@apache.org>.
jixuan1989 edited a comment on pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#issuecomment-885982984


   The motivation and the effect?


-- 
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] coveralls edited a comment on pull request #3606: [IOTDB-1372] delete TimeIndexLevel class and related methods in other classes

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#issuecomment-883859342


   
   [![Coverage Status](https://coveralls.io/builds/41703069/badge)](https://coveralls.io/builds/41703069)
   
   Coverage increased (+0.04%) to 68.204% when pulling **6b52437fc858fe209cfd46c0009221f61fa1f1a1 on yyt86:remove_device_set** into **79e50d651a2b488ac211009947b2a01c5244c37b on apache:master**.
   


-- 
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] yyt86 commented on pull request #3606: [IOTDB-1372] delete TimeIndexLevel class and related methods in other classes

Posted by GitBox <gi...@apache.org>.
yyt86 commented on pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#issuecomment-887927489


   > The motivation and the effect?
   
   After we submitted the PR, we realized those modifications are not valid for the current. I have reverted the changes in another commit of this PR. Thanks.


-- 
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] qiaojialin merged pull request #3606: [IOTDB-1372] delete devices field in FileTimeIndex

Posted by GitBox <gi...@apache.org>.
qiaojialin merged pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606


   


-- 
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] yyt86 commented on pull request #3606: [IOTDB-1372] delete TimeIndexLevel class and related methods in other classes

Posted by GitBox <gi...@apache.org>.
yyt86 commented on pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#issuecomment-892968590


   > What is this PR for?
   
   This PR has removed the devices filed of FileTimeIndex. And it will add an TsFileResourceManager class to deal with the degradation from DeviceTimeIndex to FileTimeIndex when the memory of timeIndex is larger than the threshold memory.


-- 
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] coveralls edited a comment on pull request #3606: [IOTDB-1372] delete devices field in FileTimeIndex

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#issuecomment-883859342


   
   [![Coverage Status](https://coveralls.io/builds/41943569/badge)](https://coveralls.io/builds/41943569)
   
   Coverage increased (+0.01%) to 68.286% when pulling **c9227cd5047a37cfc49f815269469a9a3a908cfa on yyt86:remove_device_set** into **785e316b3670ed25e0d966c00d09bf2aad4097ab on apache:master**.
   


-- 
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] yyt86 commented on pull request #3606: [IOTDB-1372] delete TimeIndexLevel class and related methods in other classes

Posted by GitBox <gi...@apache.org>.
yyt86 commented on pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#issuecomment-892968590


   > What is this PR for?
   
   This PR has removed the devices filed of FileTimeIndex. And it will add an TsFileResourceManager class to deal with the degradation from DeviceTimeIndex to FileTimeIndex when the memory of timeIndex is larger than the threshold memory.


-- 
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] coveralls edited a comment on pull request #3606: [IOTDB-1372] delete devices field in FileTimeIndex

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#issuecomment-883859342


   
   [![Coverage Status](https://coveralls.io/builds/41949358/badge)](https://coveralls.io/builds/41949358)
   
   Coverage increased (+0.02%) to 68.292% when pulling **032601c173b5e5463ee44ade83e77746e29fbfa7 on yyt86:remove_device_set** into **785e316b3670ed25e0d966c00d09bf2aad4097ab on apache:master**.
   


-- 
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] coveralls edited a comment on pull request #3606: [IOTDB-1372] delete TimeIndexLevel class and related methods in other classes

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#issuecomment-883859342


   
   [![Coverage Status](https://coveralls.io/builds/41702046/badge)](https://coveralls.io/builds/41702046)
   
   Coverage increased (+0.01%) to 68.177% when pulling **847b947c17a0271375593da748b22f0b46d13d2f on yyt86:remove_device_set** into **79e50d651a2b488ac211009947b2a01c5244c37b on apache:master**.
   


-- 
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] coveralls edited a comment on pull request #3606: [IOTDB-1372] delete TimeIndexLevel class and related methods in other classes

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#issuecomment-883859342


   
   [![Coverage Status](https://coveralls.io/builds/41612052/badge)](https://coveralls.io/builds/41612052)
   
   Coverage decreased (-0.05%) to 68.113% when pulling **d77555c477f9e0388393d194c87fdea221c38f26 on yyt86:remove_device_set** into **79e50d651a2b488ac211009947b2a01c5244c37b on apache:master**.
   


-- 
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] qiaojialin commented on a change in pull request #3606: [IOTDB-1372] delete TimeIndexLevel class and related methods in other classes

Posted by GitBox <gi...@apache.org>.
qiaojialin commented on a change in pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#discussion_r680293553



##########
File path: server/src/main/java/org/apache/iotdb/db/engine/storagegroup/timeindex/FileTimeIndex.java
##########
@@ -114,8 +81,14 @@ public void close() {
   }
 
   @Override
-  public Set<String> getDevices() {
-    return devices;
+  public Set<String> getDevices(String tsFilePath) {
+    try {
+      TsFileSequenceReader fileReader = FileReaderManager.getInstance().get(tsFilePath, true);
+      return new HashSet<>(fileReader.getAllDevices());
+    } catch (IOException e) {
+      logger.error("Can't read file {} from disk ", tsFilePath, e);

Review comment:
       It's better to throw IOException




-- 
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] coveralls edited a comment on pull request #3606: delete TimeIndexLevel class and related methods in other classes

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3606:
URL: https://github.com/apache/iotdb/pull/3606#issuecomment-883859342


   
   [![Coverage Status](https://coveralls.io/builds/41528332/badge)](https://coveralls.io/builds/41528332)
   
   Coverage increased (+0.003%) to 68.167% when pulling **fc1434140db715e939b4c6d8b4f437ace07cd463 on yyt86:remove_device_set** into **79e50d651a2b488ac211009947b2a01c5244c37b on apache:master**.
   


-- 
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