Skip to content

Improve the stress scripts#71

Open
Sanne wants to merge 3 commits intoquarkusio:mainfrom
Sanne:newScript
Open

Improve the stress scripts#71
Sanne wants to merge 3 commits intoquarkusio:mainfrom
Sanne:newScript

Conversation

@Sanne
Copy link
Copy Markdown
Member

@Sanne Sanne commented Nov 21, 2025

No description provided.

@Sanne Sanne requested a review from holly-cummins November 21, 2025 11:23
Copy link
Copy Markdown
Collaborator

@holly-cummins holly-cummins left a comment

Choose a reason for hiding this comment

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

The changes to stress.sh make a lot of sense, and I think are on the right side of the 'understandability vs robustness' tradeoff.

For stress-all.sh, what's the reasoning for duplicating stress.sh rather than just calling out to it? Is it to be able to redirect just parts of it to a file?

@Sanne
Copy link
Copy Markdown
Member Author

Sanne commented Nov 21, 2025

I mostly kept them separate at this point as I assumed that you probably wouldn't want to merge stress-all ..

If you think you'd like to have both I can make sure that they share functions and tidy up a little.

Comment thread scripts/stress-all.sh
Comment on lines +12 to +23
wait_for_8080() {
echo "Waiting for port 8080..."
for ((i=0; i<30; i++)); do
# Using 127.0.0.1 is safer than localhost on macOS to avoid IPv6 ::1 mismatch
if (echo > /dev/tcp/127.0.0.1/8080) >/dev/null 2>&1; then
return 0
fi
sleep 1
done
echo "Timeout waiting for port 8080"
return 1
}
Copy link
Copy Markdown
Member Author

@Sanne Sanne Nov 24, 2025

Choose a reason for hiding this comment

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

@quintesse have a look also at this script, could be useful for your own; this one waits just enough for the frameworks to have opened port 8080. Seems like a good replacement for the "10 seconds" rule.

Also, you could then use the total time of the benchmark - including boostrap times - as a fair comparison.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@Delawen as well FYI

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks @Sanne !

@holly-cummins
Copy link
Copy Markdown
Collaborator

I mostly kept them separate at this point as I assumed that you probably wouldn't want to merge stress-all ..

If you think you'd like to have both I can make sure that they share functions and tidy up a little.

Let's bring them all. Otherwise i suspect we might end up locally re-inventing that capability. :)

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.

3 participants