You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "sreejasahithi (via GitHub)" <gi...@apache.org> on 2026/04/06 06:22:34 UTC

[PR] HDDS-14938. Implement RewriteTablePath action with input validation [ozone]

sreejasahithi opened a new pull request, #10053:
URL: https://github.com/apache/ozone/pull/10053

   ## What changes were proposed in this pull request?
   This patch implements Iceberg's RewriteTablePath action and validates the inputs provided for the path rewrite.
   
   Prefix Rewrite Configuration
   Allow specifying:
     - sourcePrefix (existing path prefix)
     - targetPrefix (replacement prefix)
     - Ensure source and target prefixes are non-empty and not identical
     - Version Range Support
   
   Optional startVersion and endVersion inputs.
     - Validate that provided version exist in table.
     - Defaults:
       - endVersion → current metadata version if not provided
       - startVersion → if not set, rewrite applies up to end version from beginning version.
       
   Staging Location Handling
   - Supports user-defined staging directory.
   - If not provided, automatically generates a staging path within the tables's metadata directory.
   
   This change is required for upcoming patches in the epic: : [HDDS-14937](https://issues.apache.org/jira/browse/HDDS-14937)
   
   ## What is the link to the Apache JIRA
   [HDDS-14938](https://issues.apache.org/jira/browse/HDDS-14938)
   
   ## How was this patch tested?
   
   Green CI : https://github.com/sreejasahithi/ozone/actions/runs/24018893628
   


-- 
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@ozone.apache.org

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


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


Re: [PR] HDDS-14938. Implement Iceberg's RewriteTablePath action with input validation [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #10053:
URL: https://github.com/apache/ozone/pull/10053#discussion_r3039212042


##########
hadoop-ozone/iceberg/src/main/java/org/apache/hadoop/ozone/iceberg/RewriteTablePathOzoneAction.java:
##########
@@ -206,8 +194,17 @@ private String getMetadataLocation(Table tbl) {
       metadataDir = currentMetadataPath.substring(0, lastIndex + 1);
     }
 
-    Preconditions.checkArgument(
-        !metadataDir.isEmpty(), "Failed to get the metadata file root directory");
+    if (metadataDir.isEmpty()) {
+      throw new IllegalArgumentException("Failed to get the metadata file root directory");
+    }
     return metadataDir;
   }
+
+  private static void checkNonNullNonEmpty(String value, String messageFormat) {
+    String message = String.format(messageFormat, value);
+    Objects.requireNonNull(value, message);
+    if (value.trim().isEmpty()) {
+      throw new IllegalArgumentException(message);
+    }

Review Comment:
   Avoid eager formatting.  Actually, `value` is always either `null` or `""`, and we have separate checks for each.
   
   Suggested:
   
   ```java
   private static void checkNonNullNonEmpty(String value, String name) {
     Objects.requireNonNull(value, () -> name + " == null");
     if (value.trim().isEmpty()) {
       throw new IllegalArgumentException(name + " is empty");
     }
   }
   ```
   
   and pass only name e.g. `Source prefix`.



-- 
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@ozone.apache.org

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


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


Re: [PR] HDDS-14938. Implement Iceberg's RewriteTablePath action with input validation [ozone]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #10053:
URL: https://github.com/apache/ozone/pull/10053#issuecomment-4320265906

   This seems to have checkstyle warnings; filed HDDS-15116


-- 
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@ozone.apache.org

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


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


Re: [PR] HDDS-14938. Implement Iceberg's RewriteTablePath action with input validation [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #10053:
URL: https://github.com/apache/ozone/pull/10053#issuecomment-4198527679

   > we would have to change the existing hadoop-ozone and root pom.xml and also add in 'hadoop-ozone/dist/src/main/assemblies/ozone-src.xml'.
   
   At least the following scripts would also need to be tweaked:
   
   - `dev-support/ci/selective_ci_checks.sh` and its test
   - `hadoop-ozone/dev-support/checks/coverage.sh`
   - `hadoop-ozone/dev-support/checks/findbugs.sh`
   
   I don't see any reason to do increase the scope of this change with 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.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

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


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


Re: [PR] HDDS-14938. Implement Iceberg's RewriteTablePath action with input validation [ozone]

Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on PR #10053:
URL: https://github.com/apache/ozone/pull/10053#issuecomment-4198263944

   > > Can we put this into new module something like ozone-iceberg/tools instead of placing inside hadoop-ozone.
   > 
   > That would not fit existing build setup, why would we do that?
   
   Since this is specific just for iceberg ozone integration and not directly related to ozone module, so i was thinking not to put inside hadoop-ozone. If it doesn't fit in existing build setup then it is fine to put inside hadoop-ozone then.


-- 
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@ozone.apache.org

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


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


Re: [PR] HDDS-14938. Implement Iceberg's RewriteTablePath action with input validation [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #10053:
URL: https://github.com/apache/ozone/pull/10053#issuecomment-4206335659

   @peterxcli would you like to take 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.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

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


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


Re: [PR] HDDS-14938. Implement Iceberg's RewriteTablePath action with input validation [ozone]

Posted by "sreejasahithi (via GitHub)" <gi...@apache.org>.
sreejasahithi commented on PR #10053:
URL: https://github.com/apache/ozone/pull/10053#issuecomment-4222102059

   Thanks @adoroszlai , @ashishkumar50 and @peterxcli 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@ozone.apache.org

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


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


Re: [PR] HDDS-14938. Implement Iceberg's RewriteTablePath action with input validation [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #10053:
URL: https://github.com/apache/ozone/pull/10053#discussion_r3038447991


##########
hadoop-ozone/iceberg/src/main/java/org/apache/hadoop/ozone/iceberg/RewriteTablePathOzoneAction.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.hadoop.ozone.iceberg;
+
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import com.google.common.base.Preconditions;
+import org.apache.iceberg.HasTableOperations;
+import org.apache.iceberg.RewriteTablePathUtil;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.TableMetadata;
+import org.apache.iceberg.TableMetadata.MetadataLogEntry;
+import org.apache.iceberg.actions.ImmutableRewriteTablePath;
+import org.apache.iceberg.actions.RewriteTablePath;
+
+public class RewriteTablePathOzoneAction implements RewriteTablePath {
+
+  private static final String RESULT_LOCATION = "file-list";
+
+  private String sourcePrefix;
+  private String targetPrefix;
+  private String startVersionName;
+  private String endVersionName;
+  private String stagingDir;
+  private int parallelism = Runtime.getRuntime().availableProcessors();

Review Comment:
   Please move initialization to `RewriteTablePathOzoneAction(Table)`, since it is unnecessary for the other constructor.



##########
pom.xml:
##########
@@ -93,6 +93,7 @@
     <hk2.version>2.6.1</hk2.version>
     <httpclient.version>4.5.14</httpclient.version>
     <httpcore.version>4.4.16</httpcore.version>
+    <iceberg.version>1.10.0</iceberg.version>

Review Comment:
   Please use the latest version to save an upgrade via dependabot:
   
   ```suggestion
       <iceberg.version>1.10.1</iceberg.version>
   ```
   
   https://github.com/apache/iceberg/releases/tag/apache-iceberg-1.10.1



##########
hadoop-ozone/iceberg/pom.xml:
##########
@@ -0,0 +1,75 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  Licensed 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. See accompanying LICENSE file.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <parent>
+    <groupId>org.apache.ozone</groupId>
+    <artifactId>ozone</artifactId>
+    <version>2.2.0-SNAPSHOT</version>
+  </parent>
+  <artifactId>ozone-iceberg</artifactId>
+  <version>2.2.0-SNAPSHOT</version>
+  <packaging>jar</packaging>
+  <name>Apache Ozone Iceberg Integration</name>
+  <description>Apache Ozone Iceberg Integration</description>
+
+  <properties>
+    <classpath.skip>false</classpath.skip>
+    <maven.compiler.release>11</maven.compiler.release>
+  </properties>
+
+  <dependencies>
+
+    <!-- Common utilities -->
+    <dependency>
+      <groupId>com.google.guava</groupId>
+      <artifactId>guava</artifactId>
+    </dependency>
+
+    <!-- Iceberg dependencies -->
+    <dependency>
+      <groupId>org.apache.iceberg</groupId>
+      <artifactId>iceberg-api</artifactId>
+      <version>${iceberg.version}</version>

Review Comment:
   Please import `iceberg-bom` in root `pom.xml`, similar to:
   
   https://github.com/apache/ozone/blob/994a8abaafcb294748f594a30d4de75d95503b5c/pom.xml#L258-L264
   
   then we can remove `<version>` here.



##########
hadoop-ozone/iceberg/pom.xml:
##########
@@ -0,0 +1,75 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  Licensed 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. See accompanying LICENSE file.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <parent>
+    <groupId>org.apache.ozone</groupId>
+    <artifactId>ozone</artifactId>
+    <version>2.2.0-SNAPSHOT</version>
+  </parent>
+  <artifactId>ozone-iceberg</artifactId>
+  <version>2.2.0-SNAPSHOT</version>
+  <packaging>jar</packaging>
+  <name>Apache Ozone Iceberg Integration</name>
+  <description>Apache Ozone Iceberg Integration</description>
+
+  <properties>
+    <classpath.skip>false</classpath.skip>
+    <maven.compiler.release>11</maven.compiler.release>
+  </properties>
+
+  <dependencies>
+
+    <!-- Common utilities -->
+    <dependency>
+      <groupId>com.google.guava</groupId>
+      <artifactId>guava</artifactId>
+    </dependency>
+
+    <!-- Iceberg dependencies -->
+    <dependency>
+      <groupId>org.apache.iceberg</groupId>
+      <artifactId>iceberg-api</artifactId>
+      <version>${iceberg.version}</version>
+    </dependency>
+    <dependency>
+      <groupId>org.apache.iceberg</groupId>
+      <artifactId>iceberg-core</artifactId>
+      <version>${iceberg.version}</version>
+    </dependency>
+  </dependencies>
+
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-compiler-plugin</artifactId>
+        <configuration>
+          <!-- Iceberg 1.10+ API is Java 11 bytecode, override parent Java 8 default. -->
+          <release>11</release>

Review Comment:
   `maven.compiler.release` is also defined, do we need this?
   
   On the other hand, please disable annotation processing:
   
   ```
   <proc>none</proc>
   ```



##########
hadoop-ozone/pom.xml:
##########
@@ -106,6 +106,15 @@
   </build>
 
   <profiles>
+    <profile>
+      <id>jdk11-with-iceberg</id>
+      <activation>
+        <jdk>[11,)</jdk>
+      </activation>
+      <modules>
+        <module>iceberg</module>
+      </modules>
+    </profile>
     <profile>
       <id>build-with-ozonefs</id>

Review Comment:
   To include `ozone-iceberg` in the binary distribution, we need to add it as dependency in `ozone-dist`, similar to filesystem jars:
   
   https://github.com/apache/ozone/blob/994a8abaafcb294748f594a30d4de75d95503b5c/hadoop-ozone/dist/pom.xml#L258-L277



##########
hadoop-ozone/iceberg/pom.xml:
##########
@@ -0,0 +1,75 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  Licensed 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. See accompanying LICENSE file.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <parent>
+    <groupId>org.apache.ozone</groupId>
+    <artifactId>ozone</artifactId>
+    <version>2.2.0-SNAPSHOT</version>
+  </parent>
+  <artifactId>ozone-iceberg</artifactId>
+  <version>2.2.0-SNAPSHOT</version>
+  <packaging>jar</packaging>
+  <name>Apache Ozone Iceberg Integration</name>
+  <description>Apache Ozone Iceberg Integration</description>
+
+  <properties>
+    <classpath.skip>false</classpath.skip>
+    <maven.compiler.release>11</maven.compiler.release>
+  </properties>
+
+  <dependencies>
+
+    <!-- Common utilities -->
+    <dependency>
+      <groupId>com.google.guava</groupId>
+      <artifactId>guava</artifactId>
+    </dependency>

Review Comment:
   Let's not add `guava`, currently only used for `Preconditions`.



##########
hadoop-ozone/iceberg/src/main/java/org/apache/hadoop/ozone/iceberg/RewriteTablePathOzoneAction.java:
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.hadoop.ozone.iceberg;
+
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import com.google.common.base.Preconditions;
+import org.apache.iceberg.HasTableOperations;
+import org.apache.iceberg.RewriteTablePathUtil;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.TableMetadata;
+import org.apache.iceberg.TableMetadata.MetadataLogEntry;
+import org.apache.iceberg.actions.ImmutableRewriteTablePath;
+import org.apache.iceberg.actions.RewriteTablePath;
+
+public class RewriteTablePathOzoneAction implements RewriteTablePath {
+
+  private static final String RESULT_LOCATION = "file-list";
+
+  private String sourcePrefix;
+  private String targetPrefix;
+  private String startVersionName;
+  private String endVersionName;
+  private String stagingDir;
+  private int parallelism = Runtime.getRuntime().availableProcessors();
+
+  private final Table table;
+  private ExecutorService executorService;
+
+  public RewriteTablePathOzoneAction(Table table) {
+    this.table = table;
+  }
+
+  public RewriteTablePathOzoneAction(Table table, int parallelism) {
+    this.table = table;
+    this.parallelism = parallelism;
+  }
+
+  @Override
+  public RewriteTablePath rewriteLocationPrefix(String sPrefix, String tPrefix) {
+    Preconditions.checkArgument(
+        sPrefix != null && !sPrefix.isEmpty(), "Source prefix('%s') cannot be empty.", sPrefix);

Review Comment:
   Please replace `Preconditions.checkArgument/checkNotNull` with
   
   - `Objects.requireNonNull` (see HDDS-14057) for null-check
   - simple internal helper method for `isEmpty()` check



-- 
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@ozone.apache.org

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


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


Re: [PR] HDDS-14938. Implement Iceberg's RewriteTablePath action with input validation [ozone]

Posted by "sreejasahithi (via GitHub)" <gi...@apache.org>.
sreejasahithi commented on code in PR #10053:
URL: https://github.com/apache/ozone/pull/10053#discussion_r3039197684


##########
hadoop-ozone/pom.xml:
##########
@@ -106,6 +106,15 @@
   </build>
 
   <profiles>
+    <profile>
+      <id>jdk11-with-iceberg</id>
+      <activation>
+        <jdk>[11,)</jdk>
+      </activation>
+      <modules>
+        <module>iceberg</module>
+      </modules>
+    </profile>
     <profile>
       <id>build-with-ozonefs</id>

Review Comment:
   Right was going to add this in the upcoming PR under the Epic, but can do in this patch 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.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

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


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


Re: [PR] HDDS-14938. Implement Iceberg's RewriteTablePath action with input validation [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #10053:
URL: https://github.com/apache/ozone/pull/10053#issuecomment-4197893287

   > Can we put this into new module something like ozone-iceberg/tools instead of placing inside hadoop-ozone.
   
   That would not fit existing build setup, why would we do 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.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

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


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


Re: [PR] HDDS-14938. Implement Iceberg's RewriteTablePath action with input validation [ozone]

Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #10053:
URL: https://github.com/apache/ozone/pull/10053#discussion_r3055517053


##########
hadoop-ozone/iceberg/src/main/java/org/apache/hadoop/ozone/iceberg/RewriteTablePathOzoneAction.java:
##########
@@ -0,0 +1,209 @@
+/*
+ * 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.hadoop.ozone.iceberg;
+
+import java.util.Objects;
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import org.apache.iceberg.HasTableOperations;
+import org.apache.iceberg.RewriteTablePathUtil;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.TableMetadata;
+import org.apache.iceberg.TableMetadata.MetadataLogEntry;
+import org.apache.iceberg.actions.ImmutableRewriteTablePath;
+import org.apache.iceberg.actions.RewriteTablePath;
+
+public class RewriteTablePathOzoneAction implements RewriteTablePath {
+
+  private static final String RESULT_LOCATION = "file-list";
+
+  private String sourcePrefix;
+  private String targetPrefix;
+  private String startVersionName;
+  private String endVersionName;
+  private String stagingDir;
+  private int parallelism;
+
+  private final Table table;
+  private ExecutorService executorService;
+
+  public RewriteTablePathOzoneAction(Table table) {
+    this.table = table;
+    this.parallelism = Runtime.getRuntime().availableProcessors();
+  }
+
+  public RewriteTablePathOzoneAction(Table table, int parallelism) {
+    this.table = table;
+    this.parallelism = parallelism;
+  }
+
+  @Override
+  public RewriteTablePath rewriteLocationPrefix(String sPrefix, String tPrefix) {
+    checkNonNullNonEmpty(sPrefix, "Source prefix");
+    checkNonNullNonEmpty(tPrefix, "Target prefix");
+    this.sourcePrefix = sPrefix;
+    this.targetPrefix = tPrefix;
+    return this;
+  }
+
+  @Override
+  public RewriteTablePath startVersion(String sVersion) {
+    checkNonNullNonEmpty(sVersion, "Start version");
+    this.startVersionName = sVersion;
+    return this;
+  }
+
+  @Override
+  public RewriteTablePath endVersion(String eVersion) {
+    checkNonNullNonEmpty(eVersion, "End version");
+    this.endVersionName = eVersion;
+    return this;
+  }
+
+  @Override
+  public RewriteTablePath stagingLocation(String stagingLocation) {
+    checkNonNullNonEmpty(stagingLocation, "Staging location");
+    this.stagingDir = stagingLocation;
+    return this;
+  }
+
+  @Override
+  public Result execute() {
+    validateInputs();
+    this.executorService = Executors.newFixedThreadPool(parallelism);
+    try {
+      return doExecute();
+    } finally {

Review Comment:
   ```suggestion
       } finally {
         awaitTermination(60, TimeUnit.SECONDS);
   ```



-- 
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@ozone.apache.org

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


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


Re: [PR] HDDS-14938. Implement Iceberg's RewriteTablePath action with input validation [ozone]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #10053:
URL: https://github.com/apache/ozone/pull/10053#issuecomment-4320362014

   @sreejasahithi , thanks for the info!


-- 
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@ozone.apache.org

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


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


Re: [PR] HDDS-14938. Implement Iceberg's RewriteTablePath action with input validation [ozone]

Posted by "sreejasahithi (via GitHub)" <gi...@apache.org>.
sreejasahithi commented on PR #10053:
URL: https://github.com/apache/ozone/pull/10053#issuecomment-4320294213

   > This seems to have checkstyle warnings; filed [HDDS-15116](https://issues.apache.org/jira/browse/HDDS-15116)
   
   The checkstyle issues have already been fixed in #10067


-- 
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@ozone.apache.org

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


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


Re: [PR] HDDS-14938. Implement Iceberg's RewriteTablePath action with input validation [ozone]

Posted by "sreejasahithi (via GitHub)" <gi...@apache.org>.
sreejasahithi commented on PR #10053:
URL: https://github.com/apache/ozone/pull/10053#issuecomment-4198417586

   > > > Can we put this into new module something like ozone-iceberg/tools instead of placing inside hadoop-ozone.
   > > 
   > > 
   > > That would not fit existing build setup, why would we do that?
   > 
   > Since this is specific just for iceberg ozone integration and not directly related to ozone module, so i was thinking not to put inside hadoop-ozone. If it doesn't fit in existing build setup then it is fine to put inside hadoop-ozone then.
   
   If we want to move the module outside hadoop-ozone then we would have to change the existing hadoop-ozone and root pom.xml and also add in 'hadoop-ozone/dist/src/main/assemblies/ozone-src.xml'.
   Do we want to proceed with this or keep it in hadoop-ozone?
   @adoroszlai and @ashishkumar50 


-- 
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@ozone.apache.org

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


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


Re: [PR] HDDS-14938. Implement Iceberg's RewriteTablePath action with input validation [ozone]

Posted by "sreejasahithi (via GitHub)" <gi...@apache.org>.
sreejasahithi commented on PR #10053:
URL: https://github.com/apache/ozone/pull/10053#issuecomment-4215779613

   > 
   > If I understand correctly, RewriteTablePath is needed because when we use the ‎⁠ofs://⁠ protocol as the warehouse URL, the manifest files store data file paths with that ‎⁠ofs://⁠ prefix. An Iceberg reader configured with an S3 endpoint then can’t resolve those paths.
   
   Yes, that’s correct.
   When an Iceberg table is created in an Ozone volume/bucket using an ofs:// warehouse path, all file references stored across the table’s metadata hierarchy are written as absolute ofs:// paths. This includes:
   
     - Table metadata files (table location, manifest-list location, previous metadata file locations)
     - Manifest list files (pointing to manifest files)
     - Manifest files (pointing to data files)
     - Position delete files (referencing affected data files)
   
   Because these paths are fully qualified with the ofs:// scheme, any engine or catalog (such as Polaris) that only understands s3:// or s3a:// cannot resolve them. As a result, it cannot access the table data.
   To make the table accessible via S3-compatible systems without copying data, we need a mechanism (like RewriteTablePath) to rewrite these embedded paths from ofs:// to s3://.
   
   > Could you add an example to the Jira epic description, maybe using Apache Polaris, to walk through an end-to-end scenario? For example: create a catalog with an ‎⁠ofs://⁠ warehouse, insert some data, inspect the manifest and data file locations, show how an Iceberg S3 reader fails to read the table, and then demonstrate how the table path rewrite fixes it.
   
   sure, will update the epic description for better clarity.
   
   


-- 
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@ozone.apache.org

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


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


Re: [PR] HDDS-14938. Implement Iceberg's RewriteTablePath action with input validation [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #10053:
URL: https://github.com/apache/ozone/pull/10053#issuecomment-4222088934

   Thanks @sreejasahithi for the patch, @ashishkumar50, @peterxcli 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@ozone.apache.org

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


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


Re: [PR] HDDS-14938. Implement Iceberg's RewriteTablePath action with input validation [ozone]

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


-- 
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@ozone.apache.org

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


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