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

Fix flakiness of testQueryFiltering due to error bound (#4789) #7316

Closed
wants to merge 72 commits into from

Conversation

austintlee
Copy link
Contributor

Description

This change addresses an inappropriate/incorrect error bound on TDigest estimates on medians used in MedianAbsoluteDeviationAggregatorTests. TDigest algorithm's error is proportional to q*(1-q) where q is the percentile and is at worst in the middle (median). 0.1 is more appropriate for 90 percentile (q = 0.9).

Issues Resolved

#4789

Check List

  • New functionality includes testing.
    • [x ] All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • [ x] Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@austintlee
Copy link
Contributor Author

This is what I got from console output:

  • What went wrong:
    Execution failed for task ':distribution:bwc:minor:buildBwcLinuxTar'.

Building 2.7.0 didn't generate expected file /var/jenkins/workspace/gradle-check/search/distribution/bwc/minor/build/bwc/checkout-2.x/distribution/archives/linux-tar/build/distributions/opensearch-min-2.7.0-SNAPSHOT-linux-x64.tar.gz

I am pretty sure this is not caused by my change. How do I kick off the Gradle check again?

@andrross
Copy link
Member

@austintlee can you rebase this PR with the latest from main? I think will fix the error you’re seeing.

mingshl and others added 5 commits April 27, 2023 22:14
* Add FlatObject FieldMapper

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* resolve import package for HttpHost

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* Dynamic Create FlatObjectFieldType for dotpath field

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* Rename flat-object to flat_object and fix CI tests

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* Organized package

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* resolved compile error

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* organize package

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* Add integration tests and remove benchmark

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* fix IT tests

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* Skip IT tests before 2.7.0

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* Revert "Skip IT tests before 2.7.0"

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* Add more IT tests for supported queries

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* Removed license head and add tests for wildcard query

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* Add tests for array, nested-arrary, number and float

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* Upgrade FlatObjectFieldMapperTests to  MapperTestCase

- Upgrading `FlatObjectFieldMapperTests` from `MapperServiceTestCase` to `MapperTestCase`. The `MapperTestCase` explicitly forces us to:
	- Test parameter updates (empty now)
	- Explicitly specify if the field supports Meta and Boost (if not, relevant tests are automatically skipped)
- Test also the substring fields
- Add new test `testMapperServiceHasParser` to verify the new `flat_object` field mapper is present in mapper service registry
- Remove duplicated test and assertions methods
- Removed `testExistsQueryDocValuesDisabledWithNorms` as this test was not adding much. We shall reintroduce similar test later if we decide that we want to support ExistsQuery and what to do if DocValue are disabled and Norms enabled.

Signed-off-by: Lukáš Vlček <lukas.vlcek@aiven.io>

* Add exist query in FlatObjectFieldMapperTests and FlatObjectFieldDataTests

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* Add IT tests for painless query in testDocValues

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* Add filter script (Painless) test for flat_object

While it is not possible to use flat_object field in scripting filter context to access doc values (like `doc[<flat_object>.<field_x>]`) it is possible to call `doc[<flat_object>].size()` to get number of fields inside the flat_object field.

- Reorganize flat_object yaml tests into sections:
  - Mappings
  - Supported
  - Unsupported
- Scripting (Painless) yamlRest tests need to go into lang-painless module

Signed-off-by: Lukáš Vlček <lukas.vlcek@aiven.io>

* Removed Normalizer

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* removed unused codes

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* Remove non-relevant Javadoc from DynamicKeyFieldMapper

Signed-off-by: Lukáš Vlček <lukas.vlcek@aiven.io>

* Improve flat_object scripting test

Make it more obvious what the `doc[<flat_field>].size()` number represents.

Signed-off-by: Lukáš Vlček <lukas.vlcek@aiven.io>

* Add test for mapping parameters

Mapping parameters are not allowed in the initial version. This commit adds a test to demonstrate that trying to specify index/search analyzer for the flat_object field will fail.

Signed-off-by: Lukáš Vlček <lukas.vlcek@aiven.io>

* remove IndexAnalyzer from Builder

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* remove IndexAnalyzer from Builder

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

---------

Signed-off-by: Mingshi Liu <mingshl@amazon.com>
Signed-off-by: Lukáš Vlček <lukas.vlcek@aiven.io>
Co-authored-by: Lukáš Vlček <lukas.vlcek@aiven.io>
…6826)

* added headers to extensions rest request

Signed-off-by: Daulet <uralskdev@gmail.com>

* added tests for extension headers request

Signed-off-by: Daulet <uralskdev@gmail.com>

* applied spotless changes

Signed-off-by: Daulet <uralskdev@gmail.com>

* moved allowList/denyList code

Signed-off-by: Daulet <uralskdev@gmail.com>

* changed warning comment text

Signed-off-by: Daulet <uralskdev@gmail.com>

* Revert "changed warning comment text"

This reverts commit 3842f9e.

Signed-off-by: Daulet <uralskdev@gmail.com>

* changed warning comment text

Signed-off-by: Daulet <uralskdev@gmail.com>

* replaced List by Set in headers deny/allow list

Signed-off-by: Daulet <uralskdev@gmail.com>

* allowList, denyList moved to class level

Signed-off-by: Daulet <uralskdev@gmail.com>

* code of headers filtering moved to method

Signed-off-by: Daulet <uralskdev@gmail.com>

* Added tests for filterHeader(), changed path to uri in ExtensionRestRequest

Signed-off-by: Daulet <uralskdev@gmail.com>

* added uri and HttpVersion to ExtensionRestRequest

Signed-off-by: Daulet <uralskdev@gmail.com>

* fixed mistake in filteredHeaders

Signed-off-by: Daulet <uralskdev@gmail.com>

* fixed syntax mistakes

Signed-off-by: Daulet <uralskdev@gmail.com>

* fixed syntax mistakes 2

Signed-off-by: Daulet <uralskdev@gmail.com>

* Collectors import added to RestSendToExtensionAction

Signed-off-by: Daulet <uralskdev@gmail.com>

* added path to ExtensionRestRequest

Signed-off-by: Daulet <uralskdev@gmail.com>

* fixed compile errors 1

Signed-off-by: Daulet <uralskdev@gmail.com>

* fixed compile errors 2

Signed-off-by: Daulet <uralskdev@gmail.com>

* corrected tests for new ExtensionRestRequest

Signed-off-by: Daulet <uralskdev@gmail.com>

---------

Signed-off-by: Daulet <uralskdev@gmail.com>
* Bump org.gradle.test-retry from 1.5.1 to 1.5.2

Bumps org.gradle.test-retry from 1.5.1 to 1.5.2.

---
updated-dependencies:
- dependency-name: org.gradle.test-retry
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* Update changelog

Signed-off-by: dependabot[bot] <support@github.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
…t/fixtures/hdfs-fixture (opensearch-project#7070)

* Bump org.apache.hadoop:hadoop-minicluster in /test/fixtures/hdfs-fixture

Bumps org.apache.hadoop:hadoop-minicluster from 3.3.4 to 3.3.5.

---
updated-dependencies:
- dependency-name: org.apache.hadoop:hadoop-minicluster
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* Update changelog

Signed-off-by: dependabot[bot] <support@github.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
peterzhuamazon and others added 27 commits April 27, 2023 22:14
…rch-project#7191)

* Check if shard is in primary mode before setLatestReplicationCheckpoint.

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

* Apply Spotless

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

* Remove assert primary mode check.

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

---------

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>
* adding negative values check for fuzziness

Signed-off-by: Sean Li <lnse@amazon.com>

* addressing comment

Signed-off-by: Sean Li <lnse@amazon.com>

* addressing comment part 2

Signed-off-by: Sean Li <lnse@amazon.com>

---------

Signed-off-by: Sean Li <lnse@amazon.com>
…-sync allocation ids (opensearch-project#7214)

* [Segment Replication] Prevent cancellation of replication tracker in-sync allocation ids

Signed-off-by: Suraj Singh <surajrider@gmail.com>

* Use mutable set

Signed-off-by: Suraj Singh <surajrider@gmail.com>

* Address review comments

Signed-off-by: Suraj Singh <surajrider@gmail.com>

---------

Signed-off-by: Suraj Singh <surajrider@gmail.com>
This is a common case and info level logging will result in a ton of
unhelpful log entries.

Signed-off-by: Andrew Ross <andrross@amazon.com>
* Adding 2.8.0 to BWC versions

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

* Update lucene version in 2.8.0

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

---------

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>
Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>
Two changes here: First, when snapshotting a searchable snapshot index
(or snapshotting a full cluster that contains searchable snapshot
indices) then we will snapshot the index metadata which includes the
pointer to the original snapshot, but skip copying the data since it
already exists in the source snapshot. Second, when restoring an index
from a snapshot that is itself a searchable snapshot index, then it must
be handled as such and restored as a searchable snapshot index.

Resolves opensearch-project#7204

Signed-off-by: Andrew Ross <andrross@amazon.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
…rch-project#6748)

* fix opensearch-project#6625

Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com>

* fix opensearch-project#6625 - reverse assertEquals params

Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com>

---------

Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com>
…rch-project#7165)

Refactors Metadata.{indices, templates, customs} member variables from
ImmutableOpenMap to use jdk Maps. Usage of these variables across the
codebase is also refactored to use jdk maps.

Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
…opensearch-project#7279)

This change removes unnecessary doc parsing currently performed on replicas by
updating applyIndexOperationOnReplicas to pass a doc id from the primary.

Signed-off-by: Marc Handalian <handalm@amazon.com>
…pensearch-project#7292)

* Revert "[Segment Replication] Enable Segment Replication Backpressure setting by default. (opensearch-project#7183)"

This reverts commit a953178.

* Add null checks.

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

* add testWithDocumentReplicationEnabledIndex().

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

---------

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>
…7290)

* Bump com.diffplug.spotless from 6.17.0 to 6.18.0

Bumps com.diffplug.spotless from 6.17.0 to 6.18.0.

---
updated-dependencies:
- dependency-name: com.diffplug.spotless
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Update changelog

Signed-off-by: dependabot[bot] <support@github.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
…earch-project#7301)

This continues the refactor of ImmutableOpenMap to j.u.Map in the
o.o.cluster package in preparation for removal of HPPC dependencies.

Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
…gment read (opensearch-project#7244)

This commit changes the EngineConfig for timeseries indexes only (e.g., indexes that use 
the @timestamp metadata field) so that a descending LeafSorter comparator is used to 
visit segments in order of most newest to oldest. For the more infrequent case that a user 
chooses to sort query results by ASC time, this would cause a search regression so the 
ContextIndexSearcher is updated to inspect the sort order from the search request and 
reverse the comparator so segments are visited in ascending order. LeafSorter behavior 
for non-timeseries indexes is left the same. 

Signed-off-by: gashutos <gashutos@amazon.com>
Signed-off-by: Chaitanya Gohel <104654647+gashutos@users.noreply.github.com>
…roject#7306)

Refactors usage of ImmutableOpenMap in IndexMetadata to java.util.Map.
This continues the removal of the HPPC dependency in core.

Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
…nsearch-project#7309)

Refactors all remaining usage of HPPC backed ImmutableOpenMap to
j.u.Map. This is the last usage of the class so ImmutableOpenMap is
completely removed in favor of java maps.

Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
@austintlee austintlee closed this Apr 28, 2023
@austintlee
Copy link
Contributor Author

Screwed up the last rebase.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      2 org.opensearch.cluster.allocation.ClusterRerouteIT.testDelayWithALargeAmountOfShards

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.