Skip to content

[server] Respect log.* options for table#3358

Open
zuston wants to merge 3 commits into
apache:mainfrom
zuston:tableOptions
Open

[server] Respect log.* options for table#3358
zuston wants to merge 3 commits into
apache:mainfrom
zuston:tableOptions

Conversation

@zuston
Copy link
Copy Markdown
Member

@zuston zuston commented May 22, 2026

Purpose

Linked issue: close #xxx

Brief change log

Tests

API and Format

Documentation

Comment on lines 112 to 434
@@ -417,6 +418,17 @@ private static void checkTieredLog(Configuration tableConf) {
}
}

private static void checkLogSegmentFileSize(Configuration tableConf) {
if (tableConf.contains(ConfigOptions.LOG_SEGMENT_FILE_SIZE)
&& tableConf.get(ConfigOptions.LOG_SEGMENT_FILE_SIZE).getBytes()
> Integer.MAX_VALUE) {
throw new InvalidConfigException(
String.format(
"Invalid configuration for %s, it must be less than or equal %d bytes.",
ConfigOptions.LOG_SEGMENT_FILE_SIZE.key(), Integer.MAX_VALUE));
}
}

private static void checkPartition(
Configuration tableConf, List<String> partitionKeys, RowType rowType) {
boolean isPartitioned = !partitionKeys.isEmpty();
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

When surfacing this part of the code, could we move the option validation logic into the option definitions themselves? For example:

public static final ConfigOption<Integer> TABLE_TIERED_LOG_LOCAL_SEGMENTS =
            key("table.log.tiered.local-segments")
                    .intType()
                    .defaultValue(2)
                    .valueValidator(v -> v > 0, "must be greater than zero")
                    .withDescription(
                            "The number of log segments to retain in local for each table when log tiered storage is enabled. "
                                    + "It must be greater that 0. The default is 2.");

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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


static {
TABLE_OPTIONS = extractConfigOptions("table.");
TABLE_OPTIONS.put(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could log.index.file-size also be supported?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

sure. this is the first phase for this PR

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.

2 participants