Skip to content

Preserve context in snapshotDeletionListeners #84089

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

Conversation

DaveCTurner
Copy link
Contributor

Closes #84036

@elasticmachine elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Feb 17, 2022
@elasticmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

Hi @DaveCTurner, I've created a changelog YAML for you.

Copy link
Contributor

@arteam arteam left a comment

Choose a reason for hiding this comment

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

LGTM!

@DaveCTurner DaveCTurner added auto-backport-and-merge auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) labels Feb 17, 2022
@DaveCTurner
Copy link
Contributor Author

Thanks both!

@elasticsearchmachine elasticsearchmachine merged commit e271450 into elastic:master Feb 17, 2022
@DaveCTurner DaveCTurner deleted the 2022-02-17-snapshotDeletionListeners-preserve-context branch February 17, 2022 10:09
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Feb 17, 2022
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Feb 17, 2022
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.1
8.0

DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Feb 17, 2022
Today the `ResultDeduplicator` may complete a collection of listeners in
contexts different from the ones in which they were submitted.
`snapshotDeletionListeners` has a similar problem. This commit makes
sure that the context is preserved in these listeners.

Backports elastic#84038, elastic#84089 and elastic#84093 to 8.0 - they could not be
backported separately due to failures.
elasticsearchmachine pushed a commit that referenced this pull request Feb 17, 2022
…84100)

Today the `ResultDeduplicator` may complete a collection of listeners in
contexts different from the ones in which they were submitted.
`snapshotDeletionListeners` has a similar problem. This commit makes
sure that the context is preserved in these listeners.

Backports #84038, #84089 and #84093 to 8.0 - they could not be
backported separately due to failures.
@jtibshirani
Copy link
Contributor

I noticed another failure several hours after the fix was merged: https://gradle-enterprise.elastic.co/s/wld2abijqrsnw. Could there still be a remaining issue?

org.elasticsearch.backwards.MixedClusterClientYamlTestSuiteIT > test {p0=snapshot.get/10_basic/Get snapshot info with metadata} FAILED
    java.lang.AssertionError: Failure at [snapshot.get/10_basic:171]: Response is missing required X-Elastic-Product response header
        at __randomizedtesting.SeedInfo.seed([4DC72EC0901648A6:C593111A3EEA255E]:0)
        at org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase.executeSection(ESClientYamlSuiteTestCase.java:489)
        at org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase.test(ESClientYamlSuiteTestCase.java:462)

weizijun added a commit to weizijun/elasticsearch that referenced this pull request Feb 18, 2022
* upstream/master: (167 commits)
  Mute FrozenSearchableSnapshotsIntegTests#testCreateAndRestorePartialSearchableSnapshot
  Mute LdapSessionFactoryTests#testSslTrustIsReloaded
  Fix spotless violation from last commit
  Mute GeoGridTilerTestCase#testGeoGridSetValuesBoundingBoxes_UnboundedGeoShapeCellValues
  Small formatting clean up (elastic#84144)
  Always re-run Feature migrations which have encountered errors (elastic#83918)
  [DOCS] Clarify `orientation` usage for WKT and GeoJSON polygons (elastic#84025)
  Group field caps response by index mapping hash (elastic#83494)
  Shrink join queries in slow log (elastic#83914)
  TSDB: Reject the nested object fields that are configured time_series_dimension (elastic#83920)
  [DOCS] Remove note about partial response from Bulk API docs (elastic#84053)
  Allow regular data streams to be migrated to tsdb data streams. (elastic#83843)
  [DOCS] Fix `ignore_unavailable` parameter definition (elastic#84071)
  Make Metadata extend AbstractCollection (elastic#83791)
  Add API specs for OpenID Connect APIs
  Revert "Clean up for superuser role name references (elastic#83627)" (elastic#84096)
  Update Lucene analysis base url (elastic#84094)
  Avoid null threadContext in ResultDeduplicator (elastic#84093)
  Use static empty store files metadata (elastic#84034)
  Preserve context in snapshotDeletionListeners (elastic#84089)
  ...

# Conflicts:
#	x-pack/plugin/rollup/build.gradle
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Feb 22, 2022
In elastic#83290 we added an assertion that Elasticsearch returns the product
header in every REST response. Unfortunately this isn't always the case,
we found bugs in a couple of released versions and fixed them in elastic#84038
and elastic#84089. With this commit we skip the new assertion in the
known-buggy versions.

Closes elastic#84036 again.
elasticsearchmachine pushed a commit that referenced this pull request Feb 22, 2022
In #83290 we added an assertion that Elasticsearch returns the product
header in every REST response. Unfortunately this isn't always the case,
we found bugs in a couple of released versions and fixed them in #84038
and #84089. With this commit we skip the new assertion in the
known-buggy versions.

Closes #84036 again.
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Feb 22, 2022
In elastic#83290 we added an assertion that Elasticsearch returns the product
header in every REST response. Unfortunately this isn't always the case,
we found bugs in a couple of released versions and fixed them in elastic#84038
and elastic#84089. With this commit we skip the new assertion in the
known-buggy versions.

Closes elastic#84036 again.
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Feb 22, 2022
In elastic#83290 we added an assertion that Elasticsearch returns the product
header in every REST response. Unfortunately this isn't always the case,
we found bugs in a couple of released versions and fixed them in elastic#84038
and elastic#84089. With this commit we skip the new assertion in the
known-buggy versions.

Closes elastic#84036 again.
elasticsearchmachine pushed a commit that referenced this pull request Feb 22, 2022
In #83290 we added an assertion that Elasticsearch returns the product
header in every REST response. Unfortunately this isn't always the case,
we found bugs in a couple of released versions and fixed them in #84038
and #84089. With this commit we skip the new assertion in the
known-buggy versions.

Closes #84036 again.
elasticsearchmachine pushed a commit that referenced this pull request Feb 22, 2022
In #83290 we added an assertion that Elasticsearch returns the product
header in every REST response. Unfortunately this isn't always the case,
we found bugs in a couple of released versions and fixed them in #84038
and #84089. With this commit we skip the new assertion in the
known-buggy versions.

Closes #84036 again.
probakowski pushed a commit to probakowski/elasticsearch that referenced this pull request Feb 23, 2022
probakowski pushed a commit to probakowski/elasticsearch that referenced this pull request Feb 23, 2022
In elastic#83290 we added an assertion that Elasticsearch returns the product
header in every REST response. Unfortunately this isn't always the case,
we found bugs in a couple of released versions and fixed them in elastic#84038
and elastic#84089. With this commit we skip the new assertion in the
known-buggy versions.

Closes elastic#84036 again.
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!) >bug :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v8.0.2 v8.1.1 v8.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] RepositoryHdfsClientYamlTestSuiteIT test {yaml=hdfs_repository_2/30_snapshot/take snapshot} failing
7 participants