-
Notifications
You must be signed in to change notification settings - Fork 361
feat: For the performance test shell script, add a prefix that can reuse the created topic #2401
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
base: main
Are you sure you want to change the base?
Conversation
tasks.withType(JavaCompile) { | ||
options.compilerArgs += ["-Xlint:-this-escape"] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disabling -Xlint:this-escape
globally is not recommended, as it suppresses critical warnings about unsafe object initialization (e.g., this
escaping constructors before full initialization). This could hide concurrency risks.
Instead, consider using @SuppressWarnings("this-escape")
locally if unavoidable, but prioritize fixing the root cause for safer code.
// Modified producer start logic | ||
Function<String, List<byte[]>> payloads = payloads(config, topics); | ||
producerService.start(payloads, config.sendRate); | ||
if (config.catchupTopicPrefix != null) { | ||
LOGGER.info("Starting catchup test with existing topics"); | ||
producerService.start(payloads, config.sendRate); | ||
} else { | ||
LOGGER.info("Starting normal test with new topics"); | ||
producerService.start(payloads, config.sendRate); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of this code modification?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- s3stream/build.gradle: Language not supported
Function<String, List<byte[]>> payloads = payloads(config, topics); | ||
producerService.start(payloads, config.sendRate); | ||
if (config.catchupTopicPrefix != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider checking if config.catchupTopicPrefix is not only non-null but also non-empty, to maintain consistency with the validation logic in PerfConfig.java.
if (config.catchupTopicPrefix != null) { | |
if (!Strings.isNullOrEmpty(config.catchupTopicPrefix)) { |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
This pull request attempts to resolve #2373 .