You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "stevenzwu (via GitHub)" <gi...@apache.org> on 2023/07/04 02:13:38 UTC

[GitHub] [iceberg] stevenzwu opened a new pull request, #7978: Flink: switch to FileScanTaskParser for JSON serialization of IcebergSourceSplit

stevenzwu opened a new pull request, #7978:
URL: https://github.com/apache/iceberg/pull/7978

   (no comment)


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] stevenzwu commented on a diff in pull request #7978: Flink: switch to FileScanTaskParser for JSON serialization of IcebergSourceSplit

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #7978:
URL: https://github.com/apache/iceberg/pull/7978#discussion_r1256860948


##########
flink/v1.17/flink/src/main/java/org/apache/iceberg/flink/source/split/IcebergSourceSplit.java:
##########
@@ -120,4 +131,56 @@ static IcebergSourceSplit deserializeV1(byte[] serialized) throws IOException {
       throw new RuntimeException("Failed to deserialize the split.", e);
     }
   }
+
+  byte[] serializeV2() throws IOException {
+    if (serializedBytesCache == null) {
+      DataOutputSerializer out = SERIALIZER_CACHE.get();
+      Collection<FileScanTask> fileScanTasks = task.tasks();
+      Preconditions.checkArgument(
+          fileOffset >= 0 && fileOffset < fileScanTasks.size(),
+          "Invalid file offset: %s. Should within the range of [0, %s)",
+          fileOffset,
+          fileScanTasks.size());
+
+      out.writeInt(fileOffset);
+      out.writeLong(recordOffset);
+      out.writeInt(fileScanTasks.size());
+
+      Iterator<FileScanTask> iter = fileScanTasks.iterator();
+      int writtenTasks = 0;
+      while (iter.hasNext()) {
+        String taskJson = FileScanTaskParser.toJson(iter.next());
+        out.writeUTF(taskJson);
+        writtenTasks += 1;
+      }
+
+      Preconditions.checkArgument(
+          writtenTasks == fileScanTasks.size(),
+          "Invalid combined scan task: collection size = %s, iterator size = %s",

Review Comment:
   totally agree. removed this unnecessary check and simplified the loop.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] stevenzwu commented on pull request #7978: Flink: switch to FileScanTaskParser for JSON serialization of IcebergSourceSplit

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on PR #7978:
URL: https://github.com/apache/iceberg/pull/7978#issuecomment-1626854200

   thanks @nastra and @pvary for the review


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] stevenzwu merged pull request #7978: Flink: switch to FileScanTaskParser for JSON serialization of IcebergSourceSplit

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu merged PR #7978:
URL: https://github.com/apache/iceberg/pull/7978


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] stevenzwu commented on pull request #7978: Flink: switch to FileScanTaskParser for JSON serialization of IcebergSourceSplit

Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on PR #7978:
URL: https://github.com/apache/iceberg/pull/7978#issuecomment-1626853942

   issue #1698 


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] nastra commented on a diff in pull request #7978: Flink: switch to FileScanTaskParser for JSON serialization of IcebergSourceSplit

Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7978:
URL: https://github.com/apache/iceberg/pull/7978#discussion_r1252578426


##########
flink/v1.17/flink/src/main/java/org/apache/iceberg/flink/source/split/IcebergSourceSplit.java:
##########
@@ -120,4 +131,56 @@ static IcebergSourceSplit deserializeV1(byte[] serialized) throws IOException {
       throw new RuntimeException("Failed to deserialize the split.", e);
     }
   }
+
+  byte[] serializeV2() throws IOException {
+    if (serializedBytesCache == null) {
+      DataOutputSerializer out = SERIALIZER_CACHE.get();
+      Collection<FileScanTask> fileScanTasks = task.tasks();
+      Preconditions.checkArgument(
+          fileOffset >= 0 && fileOffset < fileScanTasks.size(),
+          "Invalid file offset: %s. Should within the range of [0, %s)",

Review Comment:
   ```suggestion
             "Invalid file offset: %s. Should be within the range of [0, %s)",
   ```



##########
flink/v1.17/flink/src/main/java/org/apache/iceberg/flink/source/split/IcebergSourceSplit.java:
##########
@@ -120,4 +131,56 @@ static IcebergSourceSplit deserializeV1(byte[] serialized) throws IOException {
       throw new RuntimeException("Failed to deserialize the split.", e);
     }
   }
+
+  byte[] serializeV2() throws IOException {
+    if (serializedBytesCache == null) {
+      DataOutputSerializer out = SERIALIZER_CACHE.get();
+      Collection<FileScanTask> fileScanTasks = task.tasks();
+      Preconditions.checkArgument(
+          fileOffset >= 0 && fileOffset < fileScanTasks.size(),
+          "Invalid file offset: %s. Should within the range of [0, %s)",
+          fileOffset,
+          fileScanTasks.size());
+
+      out.writeInt(fileOffset);
+      out.writeLong(recordOffset);
+      out.writeInt(fileScanTasks.size());
+
+      Iterator<FileScanTask> iter = fileScanTasks.iterator();
+      int writtenTasks = 0;
+      while (iter.hasNext()) {
+        String taskJson = FileScanTaskParser.toJson(iter.next());
+        out.writeUTF(taskJson);
+        writtenTasks += 1;
+      }
+
+      Preconditions.checkArgument(
+          writtenTasks == fileScanTasks.size(),
+          "Invalid combined scan task: collection size = %s, iterator size = %s",

Review Comment:
   I'm not sure it's clear to a reader what the difference between `collection size` and `iterator size` is



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org