Skip to content

[GOBBLIN-2266] Add rename order functionality for Iceberg table replication#4202

Merged
Blazer-007 merged 2 commits into
apache:masterfrom
thisisArjit:iceberg-rename-data-before-metadata
Jun 19, 2026
Merged

[GOBBLIN-2266] Add rename order functionality for Iceberg table replication#4202
Blazer-007 merged 2 commits into
apache:masterfrom
thisisArjit:iceberg-rename-data-before-metadata

Conversation

@thisisArjit

@thisisArjit thisisArjit commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Dear Gobblin maintainers,

Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!

JIRA

Description

  • Here are some details about my PR, including screenshots (if applicable):
  • Adds an ordered, two-phase recursive rename utility intended to support Iceberg table publishing semantics where metadata/ must not become visible before the data files it references.

Changes:

Introduces HadoopUtils.renameRecursivelyOrdered(...), deferring selected directory subtrees (e.g., metadata/) to a second rename phase.
Adds Iceberg-specific predicate HadoopUtils.isIcebergMetadataDir(...) and unit tests validating ordering and failure behavior.
Switches CopyDataPublisher publishing path move to use the new ordered rename.

Tests

  • Added unit tests

Commits

  • My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an ordered, two-phase recursive rename utility intended to support Iceberg table publishing semantics where metadata/ must not become visible before the data files it references.

Changes:

  • Introduces HadoopUtils.renameRecursivelyOrdered(...), deferring selected directory subtrees (e.g., metadata/) to a second rename phase.
  • Adds Iceberg-specific predicate HadoopUtils.isIcebergMetadataDir(...) and unit tests validating ordering and failure behavior.
  • Switches CopyDataPublisher publishing path move to use the new ordered rename.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
gobblin-utility/src/main/java/org/apache/gobblin/util/HadoopUtils.java Adds ordered two-phase rename + Iceberg metadata/ predicate and shared future-draining helper.
gobblin-utility/src/test/java/org/apache/gobblin/util/HadoopUtilsTest.java Adds tests covering ordered rename sequencing, atomic subtree move case, and abort-on-data-failure behavior.
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/publisher/CopyDataPublisher.java Uses ordered rename during publish to avoid publishing Iceberg metadata before data.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +631 to +639
private static void drainFutures(Queue<Future<?>> futures) throws IOException {
while (!futures.isEmpty()) {
try {
futures.poll().get();
} catch (ExecutionException | InterruptedException ee) {
throw new IOException(ee.getCause());
}
}
}
Comment on lines +581 to +585
* Used by Iceberg distcp publishing to move the {@code metadata/} subtree only after every other file (the
* {@code data/} files and anything else) has landed, since Iceberg metadata references data files by path and must
* never become visible ahead of the data it points to. {@code deferToLastPhase} is evaluated only on directory
* nodes encountered during the descent -- not on every file -- so the {@code metadata/} subtree is identified by
* its directory name and never enumerated file-by-file.
Comment on lines 343 to +346
if (statesHelper.hasAnyCopyableFile()) {
// Targets are always absolute, so we start moving from root (will skip any existing directories).
HadoopUtils.renameRecursively(this.fs, datasetWriterOutputPath, new Path("/"));
HadoopUtils.renameRecursivelyOrdered(this.fs, datasetWriterOutputPath, new Path("/"),
HadoopUtils::isIcebergMetadataDir);
@thisisArjit thisisArjit changed the title Add rename order functionality for Iceberg table replication [GOBBLIN-2266] Add rename order functionality for Iceberg table replication Jun 19, 2026
@thisisArjit thisisArjit marked this pull request as ready for review June 19, 2026 08:07
@codecov-commenter

codecov-commenter commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.98%. Comparing base (26406a8) to head (7e7096a).
⚠️ Report is 15 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #4202      +/-   ##
============================================
+ Coverage     43.26%   51.98%   +8.71%     
- Complexity     2583     7872    +5289     
============================================
  Files           516     1413     +897     
  Lines         22087    54033   +31946     
  Branches       2505     5954    +3449     
============================================
+ Hits           9557    28087   +18530     
- Misses        11566    23579   +12013     
- Partials        964     2367    +1403     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Blazer-007 Blazer-007 merged commit d3d7399 into apache:master Jun 19, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants