Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KAFKA-19080 The constraint on segment.ms is not enforced at topic level #19371

Open
wants to merge 22 commits into
base: trunk
Choose a base branch
from

Conversation

m1a2st
Copy link
Collaborator

@m1a2st m1a2st commented Apr 4, 2025

The main reason is that we forgot setting the
TopicConfig.SEGMENT_BYTES_CONFIG at least to 1024 * 1024, thus
addressed it, and add a test for it.

@github-actions github-actions bot added triage PRs from the community tools storage Pull requests that target the storage module small Small PRs labels Apr 4, 2025
@m1a2st m1a2st changed the title KAFKA-19080 The constraint on segment.ms is not enforced at topic level [WIP]KAFKA-19080 The constraint on segment.ms is not enforced at topic level Apr 4, 2025
@github-actions github-actions bot added the core Kafka Broker label Apr 4, 2025
@m1a2st
Copy link
Collaborator Author

m1a2st commented Apr 4, 2025

Hello @junrao, @chia7712
I have a question about this problem.
In KAFKA-16368, only the ServerLogConfigs value for log.segment.bytes was modified to 1 MB. However, the TopicConfig which in LogConfig default for segment.bytes remains 14 bytes. Many tests rely on this small segment.bytes value to generate a large number of segments.

I think there are two possible approaches to resolve this:

  1. Add atLeast(1024 * 1024) validation in LogConfig, which would require fixing around 400 tests.
  2. Add a validator in LogManager#updateTopicConfig to validate the change request at runtime.

@junrao
Copy link
Contributor

junrao commented Apr 9, 2025

@m1a2st : Perhaps we could somehow allow the tests to set a small segment.bytes.

@github-actions github-actions bot removed the small Small PRs label Apr 9, 2025
@chia7712
Copy link
Member

chia7712 commented Apr 9, 2025

Maybe we can add a internal config to allow tests to define small size?

@chia7712
Copy link
Member

for example:

   this.internalSegmentSize = getString(TopicConfig.INTERNAL_SEGMENT_BYTES_CONFIG);

   ...
   
    public long segmentSize() {
        if (internalSegmentSize != null) return Long.parseLong(internalSegmentSize);
        return segmentMs;
    }

@m1a2st m1a2st changed the title [WIP]KAFKA-19080 The constraint on segment.ms is not enforced at topic level KAFKA-19080 The constraint on segment.ms is not enforced at topic level Apr 11, 2025
@m1a2st
Copy link
Collaborator Author

m1a2st commented Apr 11, 2025

Thanks for @junrao, @chia7712 comments, This PR is already for review, PTAL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clients core Kafka Broker KIP-932 Queues for Kafka kraft storage Pull requests that target the storage module streams tools triage PRs from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants