Skip to content
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

Return a copy of messages in MockMTAppender #94982

Conversation

pxsalehi
Copy link
Member

@pxsalehi pxsalehi commented Apr 3, 2023

Couldn't reproduce the failure, but in principle there is an issue there. Other options are a CopyOnWriteArrayList or just synchronizing on messages() return value. Returning a copy should be enough, since this is only used in one place for asserting some test expectations in an assertBusy.

Closes #94559

@pxsalehi pxsalehi added >test Issues or PRs that are addressing/adding tests :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. labels Apr 3, 2023
@elasticsearchmachine elasticsearchmachine added v8.8.0 Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. labels Apr 3, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@pxsalehi pxsalehi added the v8.7.0 label Apr 3, 2023
@pxsalehi
Copy link
Member Author

pxsalehi commented Apr 3, 2023

Thanks, Ievgen!

@pxsalehi pxsalehi added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport-and-merge labels Apr 3, 2023
@pxsalehi
Copy link
Member Author

pxsalehi commented Apr 3, 2023

unrelated failure. opened #94987

@pxsalehi
Copy link
Member Author

pxsalehi commented Apr 3, 2023

@elasticmachine update branch

@pxsalehi
Copy link
Member Author

pxsalehi commented Apr 3, 2023

@elasticmachine run elasticsearch-ci/bwc

@elasticsearchmachine elasticsearchmachine merged commit 9c67a2e into elastic:main Apr 3, 2023
@pxsalehi pxsalehi deleted the ps230403-fix-testMergeThreadLogging branch April 3, 2023 15:30
@pxsalehi pxsalehi added v8.7.1 and removed v8.7.0 labels Apr 3, 2023
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

The backport operation could not be completed due to the following error:

An unexpected error occurred when attempting to backport this PR.

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 94982

pxsalehi added a commit to pxsalehi/elasticsearch that referenced this pull request Apr 4, 2023
Couldn't reproduce the failure, but in principle there is an issue
there. Other options are a CopyOnWriteArrayList or just synchronizing on
`messages()` return value. Returning a copy should be enough, since this
is only used in one place for asserting some test expectations in an
`assertBusy`.

Closes elastic#94559
elasticsearchmachine pushed a commit that referenced this pull request Apr 4, 2023
Couldn't reproduce the failure, but in principle there is an issue
there. Other options are a CopyOnWriteArrayList or just synchronizing on
`messages()` return value. Returning a copy should be enough, since this
is only used in one place for asserting some test expectations in an
`assertBusy`.

Closes #94559
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. >test Issues or PRs that are addressing/adding tests v8.7.1 v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] InternalEngineTests testMergeThreadLogging failing
4 participants