Skip to content

Conversation

@pltbkd
Copy link
Contributor

@pltbkd pltbkd commented Jan 21, 2026

What changes were proposed in this pull request?

  1. Introduce a client-flink-common-tiered submodule to host the shared tiered shuffle logic.
  2. Introduce a shim layer to further unify the implementations and make version-specific changes more explicit.
  3. Unify the tests as well, so that versioned clients can simply run the common test suite with their own shims.

Why are the changes needed?

Though Celeborn now has a client-flink-common submodule, clients still have to copy a lot of code from version to version, with small but necessary changes buried in the duplicates. The tests don’t share a common implementation either. Even worse, with Flink introducing tiered shuffle from 1.20 onward, all tiered shuffle implementations must be placed inside the versioned clients to ensure that client-flink-common can be compiled against all Flink versions. This makes it much harder to evolve the tiered shuffle implementations.
For the maintenance of Flink clients in the future, we need better organization of the submodules, consolidating the implementations and tests.

Does this PR resolve a correctness bug?

No.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

The patch consolidates all current Flink client tests, and can be tested with them.

Copy link
Member

@SteNicholas SteNicholas left a comment

Choose a reason for hiding this comment

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

@pltbkd, thanks for contribution. Could you update CelebornBuild for new module to build flink celeborn client jar?

@SteNicholas SteNicholas changed the title [CELEBORN-2251] Introducing a shim layer and a common-tiered submodul… [CELEBORN-2251] Introducing a shim layer and a common-tiered submodule for Flink clients Jan 21, 2026
@SteNicholas SteNicholas force-pushed the flink-shim-0.7 branch 3 times, most recently from e677771 to 6c1a0c7 Compare January 21, 2026 12:49
Copy link
Contributor

@RexXiong RexXiong left a comment

Choose a reason for hiding this comment

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

LGTM, Great refactoring work for Flink integration! Thanks!

@SteNicholas
Copy link
Member

Thanks for great refactoring. Merged to main(v0.7.0).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants