Skip to content

Conversation

@czy006
Copy link
Contributor

@czy006 czy006 commented Oct 9, 2025

Fix tableRuntime initialize failed when not have optimizer info for table

Why are the changes needed?

Close #3810.

Brief change log

  • if restore started ams null,will be first add it

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before making a pull request

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)

@czy006 czy006 requested a review from baiyangtx October 9, 2025 07:10
@github-actions github-actions bot added the module:ams-server Ams server module label Oct 9, 2025
@czy006 czy006 requested a review from zhoujinsong October 9, 2025 07:17
Copy link
Contributor

@zhoujinsong zhoujinsong left a comment

Choose a reason for hiding this comment

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

Hi,

Thanks for the contribution!
I left a comment, PTAL.

List<TableRuntimeState> states = statesMap.get(tableRuntimeMeta.getTableId());
// if first time states will be null can't be initial table runtime
if (states == null) {
TableRuntimeState state = new TableRuntimeState();
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can just initialize an empty list here, https://github.com/apache/amoro/blob/master/amoro-ams/src/main/java/org/apache/amoro/server/table/DefaultTableRuntimeStore.java#L137 will generate required states and put them in the database.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with that.

The initialization of states should be uniformly managed by the lifecycle logic of TableRuntime, and its default values have already been defined in StateKey.

Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally, stateId and OptimizingStatus are not related. TableRuntimeState represents the persistent access interface of TableRuntime, while OptimizingStatus is part of the optimization logic of Iceberg and Mixed Iceberg. The two are unrelated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we persist tableuntime for the first time, do we need to set OptimizingStatus to IDLE by default? Or will the subsequent optimization triggers automatically supplement this status cc @baiyangtx @zhoujinsong

List<TableRuntimeState> states = statesMap.get(tableRuntimeMeta.getTableId());
// if first time states will be null can't be initial table runtime
if (states == null) {
TableRuntimeState state = new TableRuntimeState();
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with that.

The initialization of states should be uniformly managed by the lifecycle logic of TableRuntime, and its default values have already been defined in StateKey.

List<TableRuntimeState> states = statesMap.get(tableRuntimeMeta.getTableId());
// if first time states will be null can't be initial table runtime
if (states == null) {
TableRuntimeState state = new TableRuntimeState();
Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally, stateId and OptimizingStatus are not related. TableRuntimeState represents the persistent access interface of TableRuntime, while OptimizingStatus is part of the optimization logic of Iceberg and Mixed Iceberg. The two are unrelated.

@tcodehuber tcodehuber changed the title [AMORO-3810] Fix ableRuntime initialize failed when not have optimizer info for table [AMORO-3810] Fix tableRuntime initialize failed when not have optimizer info for table Oct 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:ams-server Ams server module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: TableRuntime initialize failed when not have optimizer info for table

3 participants