Skip to content

Simplified the remote ipc/cluster/echo benchmark scripts. #61

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

Merged

Conversation

pveentjer
Copy link
Contributor

Instead of using a chain of commands using &&, the script is run with -e and the now there is a sequence of commands.

The original code was pretty hard to understand and this should improve maintainability.

@pveentjer pveentjer requested a review from vyazelenko June 18, 2025 11:02
@vyazelenko
Copy link
Contributor

LGTM. What about the other scripts (MDC, Archive, Kafka, gRPC) that still use the old approach?

@pveentjer
Copy link
Contributor Author

My concern is about breaking stuff. The echo/ipc/cluster benchmarks I run frequently; so any problems are quickly found.

@vyazelenko
Copy link
Contributor

My concern is about breaking stuff. The echo/ipc/cluster benchmarks I run frequently; so any problems are quickly found.

Which means that we will have two different sets of scripts which will make code maintainability far worse. Therefore we should either do in all scripts or not do it at all.

@pveentjer pveentjer force-pushed the cleanup/remote-benchmark-scripts branch from adb371f to 36c4be5 Compare June 19, 2025 10:35
@pveentjer
Copy link
Contributor Author

I have converted the rest as well.

Instead of using a chain of commands using &&, the script is run with
-e and the now there is a sequence of commands.

The original code was pretty hard to understand and this should improve
maintainability.
@pveentjer pveentjer force-pushed the cleanup/remote-benchmark-scripts branch from 36c4be5 to feb954f Compare June 19, 2025 10:45
@vyazelenko vyazelenko merged commit fbe7eb3 into aeron-io:master Jun 19, 2025
4 checks passed
pveentjer added a commit that referenced this pull request Jun 19, 2025
vyazelenko added a commit that referenced this pull request Jun 19, 2025
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