-
Notifications
You must be signed in to change notification settings - Fork 384
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
tasks.withType(JavaCompile) { | ||
options.compilerArgs += ["-Xlint:-this-escape"] | ||
} | ||
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -93,17 +93,28 @@ private PerfCommand(PerfConfig config) { | |||||
private void run() { | ||||||
LOGGER.info("Starting perf test with config: {}", jsonStringify(config)); | ||||||
TimerUtil timer = new TimerUtil(); | ||||||
|
||||||
if (config.reset) { | ||||||
LOGGER.info("Deleting all test topics..."); | ||||||
int deleted = topicService.deleteTopics(); | ||||||
LOGGER.info("Deleted all test topics ({} in total), took {} ms", deleted, timer.elapsedAndResetAs(TimeUnit.MILLISECONDS)); | ||||||
|
||||||
// Modified topic initialization logic | ||||||
List<Topic> topics; | ||||||
if (config.catchupTopicPrefix != null && !config.catchupTopicPrefix.isEmpty()) { | ||||||
LOGGER.info("Listing existing topics with prefix {}...", config.catchupTopicPrefix); | ||||||
topics = topicService.listTopicsByPrefix(config.catchupTopicPrefix); | ||||||
LOGGER.info("Found {} existing topics for catchup test", topics.size()); | ||||||
|
||||||
if (topics.isEmpty()) { | ||||||
throw new RuntimeException("No topics found with prefix: " + config.catchupTopicPrefix); | ||||||
} | ||||||
} else { | ||||||
if (config.reset) { | ||||||
LOGGER.info("Deleting all test topics..."); | ||||||
int deleted = topicService.deleteTopics(); | ||||||
LOGGER.info("Deleted {} test topics, took {} ms", deleted, timer.elapsedAndResetAs(TimeUnit.MILLISECONDS)); | ||||||
} | ||||||
LOGGER.info("Creating {} new topics...", config.topics); | ||||||
topics = topicService.createTopics(config.topicsConfig()); | ||||||
LOGGER.info("Created {} topics, took {} ms", topics.size(), timer.elapsedAndResetAs(TimeUnit.MILLISECONDS)); | ||||||
} | ||||||
|
||||||
LOGGER.info("Creating topics..."); | ||||||
List<Topic> topics = topicService.createTopics(config.topicsConfig()); | ||||||
LOGGER.info("Created {} topics, took {} ms", topics.size(), timer.elapsedAndResetAs(TimeUnit.MILLISECONDS)); | ||||||
|
||||||
LOGGER.info("Creating consumers..."); | ||||||
int consumers = consumerService.createConsumers(topics, config.consumersConfig()); | ||||||
consumerService.start(this::messageReceived, config.maxConsumeRecordRate); | ||||||
|
@@ -117,8 +128,15 @@ private void run() { | |||||
waitTopicsReady(consumerService.consumerCount() > 0); | ||||||
LOGGER.info("Topics are ready, took {} ms", timer.elapsedAndResetAs(TimeUnit.MILLISECONDS)); | ||||||
|
||||||
// Modified producer start logic | ||||||
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 commentThe 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.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
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); | ||||||
} | ||||||
Comment on lines
+131
to
+139
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the purpose of this code modification? |
||||||
|
||||||
preparing = false; | ||||||
|
||||||
|
@@ -132,7 +150,7 @@ private void run() { | |||||
} | ||||||
|
||||||
Result result; | ||||||
if (config.backlogDurationSeconds > 0) { | ||||||
if (config.catchupTopicPrefix == null && config.backlogDurationSeconds > 0) { | ||||||
LOGGER.info("Pausing consumers for {} seconds to build up backlog...", config.backlogDurationSeconds); | ||||||
consumerService.pause(); | ||||||
long backlogStart = System.currentTimeMillis(); | ||||||
|
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.