-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Fix long-running stress test #3346
Conversation
938a6a3
to
e798914
Compare
qa/L0_long_running_stress/stress.py
Outdated
@@ -51,7 +51,7 @@ | |||
|
|||
FLAGS = None | |||
CORRELATION_ID_BLOCK_SIZE = 100 |
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.
Make CORRELATION_ID_BLOCK_SIZE much larger... 1024 * 1024
qa/L0_long_running_stress/stress.py
Outdated
# instead of being a sequence missing start flag, which gives results | ||
# not expected. | ||
if test_case_name == "sequence_no_start": | ||
sequence_id *= 10 |
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.
Just multiplying by 10 seems a little magical... I think there is a better way described below
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.
Will remove this part and use the original approach mentioned below.
a6b98c8
8a38abb
to
a6b98c8
Compare
@deadeyegoodwin I think the original logic doesn't work well is because in the original sequence stress test, we only serve one model, so we can just use the last_seq_choice to see if the last sequence case is some no-end cases. But we are serving multiple models in this stress test, so we need to remember which model is used for the no-end cases. |
No description provided.