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

[Remove] Default Mapping #2151

Merged
merged 1 commit into from
Feb 18, 2022

Conversation

nknize
Copy link
Collaborator

@nknize nknize commented Feb 17, 2022

Default mappings were deprecated in Legacy version 6x and cannot be added to
indexes created in legacy 7+ or any version of OpenSearch. All support for
default mappings are removed for OpenSearch 2+.

relates #1674

@nknize nknize added v2.0.0 Version 2.0.0 non-issue bugs / unexpected behaviors that end up non issues; audit trail simple changes that aren't issues >breaking Identifies a breaking change. Indexing & Search labels Feb 17, 2022
@nknize nknize requested a review from a team as a code owner February 17, 2022 05:04
@opensearch-ci-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 61223865898ecbb70e1c631c1b03f4cd5054ae7a
Log 2485

Reports 2485

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 27179cc38d7968cea153092e6065ab07fa5f0554
Log 2486

Reports 2486

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 9f5e29629b5014917ef3aa03b1242acb8ffb6617
Log 2490

Reports 2490

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure bea8716c2fdb4befc9ba16125b4580a3402953aa
Log 2496

Reports 2496

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 50c29d6e1b087b568515b42bbe294de6b48a9797
Log 2497

Reports 2497

@nknize nknize force-pushed the remove/defaultMapping branch from 50c29d6 to 0df7f69 Compare February 17, 2022 17:17
@nknize
Copy link
Collaborator Author

nknize commented Feb 17, 2022

Composite agg failures will be fixed by #2147

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 0df7f694c419f2af73f25ebd8f21104f5fe02913
Log 2500

Reports 2500

@nknize
Copy link
Collaborator Author

nknize commented Feb 17, 2022

related to #2147; will hold off on refiring until the transport version check is changed.

./gradlew ':qa:mixed-cluster:v1.3.0#mixedClusterTest' --tests "org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT" -Dtests.method="test {p0=search.aggregation/230_composite/Composite aggregation with date_histogram offset}" -Dtests.seed=66D8ABC9216C5F00 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ru -Dtests.timezone=Asia/Macau -Druntime.java=17

@nknize nknize force-pushed the remove/defaultMapping branch from 0df7f69 to 055b76f Compare February 17, 2022 18:42
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 055b76fa27285eafcee00af8cff0c5bd52646cf6
Log 2505

Reports 2505

@@ -474,7 +474,7 @@ public void testUpdateRelations() throws Exception {
.endObject()
);
docMapper = indexService.mapperService()
.merge("_doc", new CompressedXContent(updateMapping), MapperService.MergeReason.MAPPING_UPDATE);
Copy link
Collaborator

@reta reta Feb 17, 2022

Choose a reason for hiding this comment

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

@nknize I am a bit confused by this change: the only mapping which should left at this point is _doc, would we still let anyone to change the (single) mapping type for index (as here fe, _doc -> type)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

(I think I found the answer to my question)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

type support hasn't been completely removed yet so this will get cleaned up in a follow up.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@@ -301,10 +300,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
} else {
MappingMetadata mappings = null;
for (final ObjectObjectCursor<String, MappingMetadata> typeEntry : indexMappings) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably future change, but I believe we would significantly reduce the amount of "heap garbage" when replace ImmutableOpenMap<String, MappingMetadata> with just MappingMetadata since there is only one mapping from now on.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I think that would be a good followup PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@@ -131,7 +131,7 @@ public void testSimple() throws Exception {

public void testSimpleWithXContentTraverse() throws Exception {
String mapping = copyToStringFromClasspath("/org/opensearch/index/mapper/dynamictemplate/simple/test-mapping.json");
MapperService mapperService = createMapperService("person", mapping);
Copy link
Collaborator

Choose a reason for hiding this comment

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

That should also work with person, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No; we're in a transition state right now removing type support. I haven't fully removed types from the MapperService yet. That's coming in a followup PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@nknize nknize force-pushed the remove/defaultMapping branch 2 times, most recently from 40ad94c to d46faa1 Compare February 17, 2022 23:15
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 40ad94c03b1ac95da9cedfcfdee333f52e74ae32
Log 2536

Reports 2536

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure d46faa12ad79b7049348747bfb32e0a627ac551e
Log 2539

Reports 2539

@nknize nknize force-pushed the remove/defaultMapping branch from d46faa1 to dddb5f5 Compare February 18, 2022 00:11
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure dddb5f5f9b3be8778b42417d211adce566ae9c0a
Log 2543

Reports 2543

@nknize
Copy link
Collaborator Author

nknize commented Feb 18, 2022

le uggh.. docker failure

@nknize
Copy link
Collaborator Author

nknize commented Feb 18, 2022

start gradle check

@nknize nknize force-pushed the remove/defaultMapping branch from dddb5f5 to 7ccfb56 Compare February 18, 2022 02:15
Default mappings were deprecated in Legacy version 6x and cannot be added to
indexes created in legacy 7+ or any version of OpenSearch. All support for
default mappings are removed for OpenSearch 2+.

Signed-off-by: Nicholas Walter Knize <[email protected]>
@nknize nknize force-pushed the remove/defaultMapping branch from 7ccfb56 to 288aaba Compare February 18, 2022 02:22
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure dddb5f5f9b3be8778b42417d211adce566ae9c0a
Log 2550

Reports 2550

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 7ccfb56f9bdfd0c664308d42015409d5de029f7d
Log 2551

Reports 2551

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 288aaba
Log 2552

Reports 2552

@nknize
Copy link
Collaborator Author

nknize commented Feb 18, 2022

Unable to reproduce. Flaky failure already reported in #1828

Refiring check

@nknize
Copy link
Collaborator Author

nknize commented Feb 18, 2022

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 288aaba
Log 2556

Reports 2556

@nknize
Copy link
Collaborator Author

nknize commented Feb 18, 2022

Same flaky failure already reported in #1957

Refiring!

./gradlew ':server:test' --tests "org.opensearch.cluster.routing.MovePrimaryFirstTests.testClusterGreenAfterPartialRelocation" -Dtests.seed=1B2C0BA5EB846A5F -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ar-TN -Dtests.timezone=Asia/Yakutsk -Druntime.java=17

@nknize
Copy link
Collaborator Author

nknize commented Feb 18, 2022

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 288aaba
Log 2558

Reports 2558

@nknize nknize merged commit b75d3f1 into opensearch-project:main Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>breaking Identifies a breaking change. Indexing & Search non-issue bugs / unexpected behaviors that end up non issues; audit trail simple changes that aren't issues v2.0.0 Version 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants