Skip to content

MINOR: Raft max batch size needs to propagate to log config #10256

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.

Sign up for GitHub

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

Merged
hachikuji merged 3 commits into from
Mar 4, 2021
Merged

MINOR: Raft max batch size needs to propagate to log config #10256

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c10f840
MINOR: Raft max batch size needs to propagate to log config
hachikuji Mar 4, 2021
dfdd4a3
Test cases for max batch size
hachikuji Mar 4, 2021
fad6688
Remove second `apply` and add helper for snapshot recovery
hachikuji Mar 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions core/src/main/scala/kafka/raft/RaftManager.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,8 @@ class KafkaRaftManager[T](

private def buildMetadataLog(): KafkaMetadataLog = {
val defaultProps = LogConfig.extractLogConfigMap(config)

@mumrah mumrah Mar 4, 2021

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 see we are no longer calling this, but rather explicitly populating the log properties. Are there other log configs we might want to expose?

@hachikuji hachikuji Mar 4, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It seemed best for now to not allow overrides since this is a new usage of Log and we haven't had time to understand the impact of all of the configurations for this usage. Most of them are probably safe, but others do not even make sense (e.g. the retention settings). Let me open a JIRA so that we can consider which configs we want to expose for the metadata log and how we want to expose them.

@hachikuji hachikuji Mar 4, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I filed this JIRA: https://issues.apache.org/jira/browse/KAFKA-12423.

defaultProps.put(LogConfig.MaxMessageBytesProp, KafkaRaftClient.MAX_BATCH_SIZE: java.lang.Integer)

LogConfig.validateValues(defaultProps)
val defaultLogConfig = LogConfig(defaultProps)

Expand Down
2 changes: 1 addition & 1 deletion raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@
public class KafkaRaftClient<T> implements RaftClient<T> {
private static final int RETRY_BACKOFF_BASE_MS = 100;
private static final int FETCH_MAX_WAIT_MS = 500;
static final int MAX_BATCH_SIZE = 8 * 1024 * 1024;
public static final int MAX_BATCH_SIZE = 8 * 1024 * 1024;

private final AtomicReference<GracefulShutdown> shutdown = new AtomicReference<>();
private final Logger logger;
Expand Down