close
Skip to content

[client] Get tableInfo via the Admin API instead of via metadata updater#2016

Merged
wuchong merged 8 commits into
apache:mainfrom
swuferhong:client-tableinfo-fix
Nov 29, 2025
Merged

[client] Get tableInfo via the Admin API instead of via metadata updater#2016
wuchong merged 8 commits into
apache:mainfrom
swuferhong:client-tableinfo-fix

Conversation

@swuferhong
Copy link
Copy Markdown
Contributor

Purpose

Linked issue: close #2014

Get TableInfo via the Admin API instead of via metadata updater. This PR does not aim to fully remove TableInfo from the metadataUpdater and the Cluster—that work will be done in 483. This PR only changes the way table info is retrieved.

Brief change log

Tests

API and Format

Documentation

@wuchong
Copy link
Copy Markdown
Member

wuchong commented Nov 26, 2025

@swuferhong is this PR ready to review? It seems there are tests are failing.

Copy link
Copy Markdown
Member

@wuchong wuchong left a comment

Choose a reason for hiding this comment

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

We can put TableInfo into WriteRecord, and then we don't need TableInfoCache.

Comment thread fluss-common/src/main/java/org/apache/fluss/cluster/Cluster.java Outdated
@swuferhong swuferhong force-pushed the client-tableinfo-fix branch 3 times, most recently from 73a02b6 to fa9445a Compare November 28, 2025 07:16
Copy link
Copy Markdown
Member

@wuchong wuchong left a comment

Choose a reason for hiding this comment

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

@swuferhong I made a little changes to the PR and addressed my comments. Please take a look.


@Override
public int hashCode() {
return Objects.hash(tableId);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The TableKey is super hack, because it is used as a map key, but the TableInfo is not considered for comparison. If we want to carry the TableInfo in the Map, it should belong to the values. However, I think we don't need the TableInfo to check log/kv. I made a change for this.

@AfterAll
protected static void afterAll() throws Exception {
conn.close();
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we can revert all the test changes.

int bucketId,
PhysicalTablePath physicalTablePath,
int schemaId,
TableInfo tableInfo,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should avoid passing large context objects; pass only the required data.

@wuchong wuchong merged commit c5257fd into apache:main Nov 29, 2025
5 checks passed
Ugbot pushed a commit to Ugbot/fluss that referenced this pull request Apr 26, 2026
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.

FlussConnection getTable method fails due to missing retries when fetching metadata

2 participants