Truncate errors at 256 characters, fix flaky test. #556
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dblock
requested review from
harshavamsi,
sachetalva,
nhtruong,
Xtansia,
VachaShah,
Tokesh and
aabeshov
as code owners
dblock
force-pushed
the
truncate-at-256
branch
from
0c649f6
to
c186bfc
Compare
Changes AnalysisCommit SHA: 6a0f9c4 API ChangesSummaryNO CHANGES ReportThe full API changes report is available at: https://github.com/opensearch-project/opensearch-api-specification/actions/runs/10724577950/artifacts/1897365853 API Coverage
|
dblock
force-pushed
the
truncate-at-256
branch
from
c186bfc
to
cf14a2a
Compare
Spec Test Coverage Analysis
|
dblock
force-pushed
the
truncate-at-256
branch
from
6a91120
to
848e142
Compare
dblock
changed the title
dblock
commented
Sep 5, 2024
| @@ -51,5 +63,8 @@ chapters: | |||
| shard: 0 | |||
| from_node: opensearch-node1 | |||
| to_node: opensearch-node2 | |||
| retry: | |||
There was a problem hiding this comment.
Shard may be still relocating, retry.
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
dblock
force-pushed
the
truncate-at-256
branch
from
848e142
to
6a0f9c4
Compare
nhtruong
reviewed
Sep 5, 2024
| @@ -152,8 +152,9 @@ export class ConsoleResultLogger implements ResultLogger { | |||
| } | |||
|
|
|||
| #maybe_shorten_error_message(message: string | undefined): string | undefined { | |||
| if (message === undefined || message.length <= 128 || this._verbose) return message | |||
| const part = message.split(',')[0] | |||
| const cut_at = 256 | |||
There was a problem hiding this comment.
Nit: This is better off as a configurable number in the constructor, esp when we refactor the test framework to be a standalone product.
There was a problem hiding this comment.
I'll leave it as is for now, but we can expose it as an option when someone actually wants to change this value programmatically.
nhtruong
approved these changes
Sep 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
I was trying to debug #555 and couldn't reproduce it locally. I wish I saw the error in CI, but we're being too clever with truncation of the error message. Be less clever in this PR, which produces the follow output.
The relocation test is flaky because sometimes the shard is allocated on
opensearch-node-1and sometimes on-2.Three possible fixes:
moviesindex is allocated, then relocate it to the other one. The problem is to know what this "other" node is in the test.index.routing.allocation.include.zone: zoneAwhich would put the shard on node1, disabling rebalance, then removing that setting.node1in a prologue and ignore errors.I chose (3) because it's simpler to think about.
Closes #555.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.