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 2022/09/30 08:54:02 UTC

[GitHub] [iotdb] JackieTien97 commented on a diff in pull request #7495: [IOTDB-4562] Optimization of ITimeIndex

JackieTien97 commented on code in PR #7495:
URL: https://github.com/apache/iotdb/pull/7495#discussion_r984364158


##########
server/src/main/java/org/apache/iotdb/db/engine/storagegroup/timeindex/FileTimeIndex.java:
##########
@@ -88,23 +82,28 @@ public void close() {
 
   @Override
   public Set<String> getDevices(String tsFilePath, TsFileResource tsFileResource) {
-    FILE_READER_MANAGER.increaseFileReaderReference(tsFileResource, tsFileResource.isClosed());
-    try {
-      TsFileSequenceReader fileReader = FileReaderManager.getInstance().get(tsFilePath, true);
-      return new HashSet<>(fileReader.getAllDevices());
+    try (InputStream inputStream =

Review Comment:
   ```suggestion
       tsFileResource.readLock();
       try (InputStream inputStream =
   ```



##########
server/src/main/java/org/apache/iotdb/db/engine/storagegroup/timeindex/FileTimeIndex.java:
##########
@@ -88,23 +82,28 @@ public void close() {
 
   @Override
   public Set<String> getDevices(String tsFilePath, TsFileResource tsFileResource) {
-    FILE_READER_MANAGER.increaseFileReaderReference(tsFileResource, tsFileResource.isClosed());
-    try {
-      TsFileSequenceReader fileReader = FileReaderManager.getInstance().get(tsFilePath, true);
-      return new HashSet<>(fileReader.getAllDevices());
+    try (InputStream inputStream =
+        FSFactoryProducer.getFSFactory()
+            .getBufferedInputStream(tsFilePath + TsFileResource.RESOURCE_SUFFIX)) {
+      // The first byte is VERSION_NUMBER, second byte is timeIndexType.
+      ReadWriteIOUtils.readBytes(inputStream, 2);
+      ITimeIndex timeIndex = new DeviceTimeIndex().deserialize(inputStream);
+      return timeIndex.getDevices(tsFilePath, tsFileResource);

Review Comment:
   Better to add a method in DeviceTimeIndex to just get useful info i.e. deviceName. In the new method, you don't need to  do like `ReadWriteIOUtils.readLong(inputStream)`  operations which not only read 8 bytes but also call `BytesUtils.bytesToLong(bytes);` to convert it into long and this is unnecessary.



##########
server/src/main/java/org/apache/iotdb/db/engine/storagegroup/timeindex/FileTimeIndex.java:
##########
@@ -88,23 +82,28 @@ public void close() {
 
   @Override
   public Set<String> getDevices(String tsFilePath, TsFileResource tsFileResource) {
-    FILE_READER_MANAGER.increaseFileReaderReference(tsFileResource, tsFileResource.isClosed());
-    try {
-      TsFileSequenceReader fileReader = FileReaderManager.getInstance().get(tsFilePath, true);
-      return new HashSet<>(fileReader.getAllDevices());
+    try (InputStream inputStream =
+        FSFactoryProducer.getFSFactory()
+            .getBufferedInputStream(tsFilePath + TsFileResource.RESOURCE_SUFFIX)) {
+      // The first byte is VERSION_NUMBER, second byte is timeIndexType.
+      ReadWriteIOUtils.readBytes(inputStream, 2);
+      ITimeIndex timeIndex = new DeviceTimeIndex().deserialize(inputStream);
+      return timeIndex.getDevices(tsFilePath, tsFileResource);
     } catch (NoSuchFileException e) {
       // deleted by ttl
       if (tsFileResource.isDeleted()) {
         return Collections.emptySet();
       } else {
-        logger.error("Can't read file {} from disk ", tsFilePath, e);
-        throw new RuntimeException("Can't read file " + tsFilePath + " from disk");
+        logger.error(
+            "Can't read file {} from disk ", tsFilePath + TsFileResource.RESOURCE_SUFFIX, e);
+        throw new RuntimeException(
+            "Can't read file " + tsFilePath + TsFileResource.RESOURCE_SUFFIX + " from disk");
       }
     } catch (Exception e) {
-      logger.error("Failed to get devices from tsfile: {}", tsFilePath, e);
-      throw new RuntimeException("Failed to get devices from tsfile:: " + tsFilePath);
-    } finally {
-      FILE_READER_MANAGER.decreaseFileReaderReference(tsFileResource, tsFileResource.isClosed());
+      logger.error(
+          "Failed to get devices from tsfile: {}", tsFilePath + TsFileResource.RESOURCE_SUFFIX, e);
+      throw new RuntimeException(
+          "Failed to get devices from tsfile:: " + tsFilePath + TsFileResource.RESOURCE_SUFFIX);
     }

Review Comment:
   ```suggestion
       } finally {
         tsFileResource.readUnlock();
       }
   ```



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