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] Multiple Types from IndexTemplateMetadata #2400

Merged

Conversation

nknize
Copy link
Collaborator

@nknize nknize commented Mar 8, 2022

Removes multi-type support from IndexTemplateMetadata so that instead of holding
a map of multiple types to mappings, it only returns a single mapping for a
single type. Also removes type from documentMapper() method to avoid any
accidental NullPointerExceptions in the internal mapping retrieval.

relates #1940
closes #2359

@nknize nknize added v2.0.0 Version 2.0.0 >breaking Identifies a breaking change. Indexing & Search labels Mar 8, 2022
@nknize nknize requested a review from a team as a code owner March 8, 2022 06:12
@opensearch-ci-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 1a261f7534000cc101e77471943f025c24d9563b
Log 3157

Reports 3157

@nknize nknize force-pushed the mappings/singleForIndexTemplateMetadata branch from 1a261f7 to 1342608 Compare March 8, 2022 18:17
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 1342608c99734d28087cff11f503394b7d6507b0
Log 3176

Reports 3176

@nknize nknize force-pushed the mappings/singleForIndexTemplateMetadata branch from 1342608 to b5cc5c2 Compare March 8, 2022 20:29
serverTemplateBuilder.putMapping(MapperService.SINGLE_MAPPING_NAME, clientITMD.mappings().source());
// The client-side mappings never include a wrapping type, but server-side mappings
// for index templates still do so we need to wrap things here
String mappings = "{\"" + MapperService.SINGLE_MAPPING_NAME + "\": " + clientITMD.mappings().source().string() + "}";
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 but will be fixed later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure b5cc5c206e47d1098d628659f93ff1f33fbefcce
Log 3185

Reports 3185

@nknize nknize force-pushed the mappings/singleForIndexTemplateMetadata branch from b5cc5c2 to b89417a Compare March 10, 2022 13:39
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure b89417a20a27f729f8d5bd01633c224478a50958
Log 3223

Reports 3223

@dreamer-89
Copy link
Member

dreamer-89 commented Mar 10, 2022

distribution:bwc:minor:buildBwcLinuxTar failure. Getting fixed in #2430

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':distribution:bwc:minor:buildBwcLinuxTar'.
> Building 1.3.0 didn't generate expected file /var/CITOOL/workflow/OpenSearch_CI/PR_Checks/Gradle_Check/search/distribution/bwc/minor/build/bwc/checkout-1.x/distribution/archives/linux-tar/build/distributions/opensearch-min-1.3.0-SNAPSHOT-linux-x64.tar.gz

@dreamer-89
Copy link
Member

start gradle check

@reta
Copy link
Collaborator

reta commented Mar 10, 2022

distribution:bwc:minor:buildBwcLinuxTar failure. Getting fixed in #2430

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':distribution:bwc:minor:buildBwcLinuxTar'.
> Building 1.3.0 didn't generate expected file /var/CITOOL/workflow/OpenSearch_CI/PR_Checks/Gradle_Check/search/distribution/bwc/minor/build/bwc/checkout-1.x/distribution/archives/linux-tar/build/distributions/opensearch-min-1.3.0-SNAPSHOT-linux-x64.tar.gz

@dreamer-89 The pull request was merged moments ago, may need rebase

@dreamer-89
Copy link
Member

distribution:bwc:minor:buildBwcLinuxTar failure. Getting fixed in #2430

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':distribution:bwc:minor:buildBwcLinuxTar'.
> Building 1.3.0 didn't generate expected file /var/CITOOL/workflow/OpenSearch_CI/PR_Checks/Gradle_Check/search/distribution/bwc/minor/build/bwc/checkout-1.x/distribution/archives/linux-tar/build/distributions/opensearch-min-1.3.0-SNAPSHOT-linux-x64.tar.gz

@dreamer-89 The pull request was merged moments ago, may need rebase

@nknize : Can you please rebase against main ?

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success b89417a20a27f729f8d5bd01633c224478a50958
Log 3231

Reports 3231

Copy link
Member

@dreamer-89 dreamer-89 left a comment

Choose a reason for hiding this comment

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

LGTM!

Thank you for taking time to raise this PR.

source = Collections.singletonMap(MapperService.SINGLE_MAPPING_NAME, source);
} else if (MapperService.SINGLE_MAPPING_NAME.equals(type) == false) {
// if it has a different type name, then unwrap and rewrap with _doc
source = Collections.singletonMap(MapperService.SINGLE_MAPPING_NAME, source.get(type));
Copy link
Member

Choose a reason for hiding this comment

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

What is the benefit of re-wrapping with _doc instead of throwing IllegalArgumentException ? Is this for handling typed indices from older versions ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For template bwc; same as above.

nknize added 4 commits March 10, 2022 19:27
Removes multi-type support from IndexTemplateMetadata so that instead of holding
a map of multiple types to mappings, it only returns a single mapping for a
single type.

Signed-off-by: Nicholas Walter Knize <[email protected]>
Signed-off-by: Nicholas Walter Knize <[email protected]>
Signed-off-by: Nicholas Walter Knize <[email protected]>
Signed-off-by: Nicholas Walter Knize <[email protected]>
@nknize nknize force-pushed the mappings/singleForIndexTemplateMetadata branch from b89417a to 16aff4e Compare March 11, 2022 01:28
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 16aff4e
Log 3244

Reports 3244

@dreamer-89
Copy link
Member

dreamer-89 commented Mar 11, 2022

This issue is getting tracked in #2359

> Task :qa:full-cluster-restart:v1.2.5#oldClusterTest

REPRODUCE WITH: ./gradlew ':qa:full-cluster-restart:v1.2.5#oldClusterTest' --tests "org.opensearch.upgrades.FullClusterRestartIT.testEmptyShard" -Dtests.seed=B30D22FAABA64229 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=es-BO -Dtests.timezone=Brazil/DeNoronha -Druntime.java=17

org.opensearch.upgrades.FullClusterRestartIT > testEmptyShard FAILED
    org.opensearch.client.WarningFailureException: method [PUT], host [http://[::1]:35241], URI [/test_empty_shard], status line [HTTP/1.1 200 OK]
    Warnings: [Translog retention settings [index.translog.retention.age] and [index.translog.retention.size] are deprecated and effectively ignored. They will be removed in a future version.]
    {"acknowledged":true,"shards_acknowledged":true,"index":"test_empty_shard"}
        at __randomizedtesting.SeedInfo.seed([B30D22FAABA64229:FF76F522579F6DBC]:0)
        at app//org.opensearch.client.RestClient.convertResponse(RestClient.java:346)
        at app//org.opensearch.client.RestClient.performRequest(RestClient.java:320)
        at app//org.opensearch.client.RestClient.performRequest(RestClient.java:295)
        at app//org.opensearch.test.rest.OpenSearchRestTestCase.createIndex(OpenSearchRestTestCase.java:973)
        at app//org.opensearch.test.rest.OpenSearchRestTestCase.createIndex(OpenSearchRestTestCase.java:956)
        at app//org.opensearch.test.rest.OpenSearchRestTestCase.createIndex(OpenSearchRestTestCase.java:952)
        at app//org.opensearch.upgrades.FullClusterRestartIT.testEmptyShard(FullClusterRestartIT.java:670)

@dreamer-89
Copy link
Member

start gradle check

@nknize
Copy link
Collaborator Author

nknize commented Mar 11, 2022

This issue is getting tracked in #2359

See #2359 (comment)

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 16aff4e
Log 3245

Reports 3245

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 13a592d
Log 3247

Reports 3247

@nknize
Copy link
Collaborator Author

nknize commented Mar 11, 2022

Another #1746; refiring

./gradlew ':server:internalClusterTest' --tests "org.opensearch.gateway.RecoveryFromGatewayIT.testReuseInFileBasedPeerRecovery" -Dtests.seed=488FED743787DA93 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=sr-Latn-ME -Dtests.timezone=America/Recife -Druntime.java=17
org.opensearch.gateway.RecoveryFromGatewayIT > testReuseInFileBasedPeerRecovery FAILED
    java.lang.AssertionError: shard [test][0] on node [node_t1] has pending operations:
     --> RetentionLeaseBackgroundSyncAction.Request{retentionLeases=RetentionLeases{primaryTerm=1, version=1591, leases={peer_recovery/KE4lkT4_Sk60Kzfrtc_G6w=RetentionLease{id='peer_recovery/KE4lkT4_Sk60Kzfrtc_G6w', retainingSequenceNumber=1646, timestamp=1646966789226, source='peer recovery'}, peer_recovery/VA3o7X03RpmgTx04G6DugA=RetentionLease{id='peer_recovery/VA3o7X03RpmgTx04G6DugA', retainingSequenceNumber=1646, timestamp=1646966789226, source='peer recovery'}}}, shardId=[test][0], timeout=1m, index='test', waitForActiveShards=0}
    	at org.opensearch.index.shard.IndexShardOperationPermits.acquire(IndexShardOperationPermits.java:248)
    	at org.opensearch.index.shard.IndexShard.acquirePrimaryOperationPermit(IndexShard.java:3208)
    	at org.opensearch.action.support.replication.TransportReplicationAction.acquirePrimaryOperationPermit(TransportReplicationAction.java:1116)
    	at org.opensearch.action.support.replication.TransportReplicationAction$AsyncPrimaryAction.doRun(TransportReplicationAction.java:433)
    	at org.opensearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:50)
    	at org.opensearch.action.support.replication.TransportReplicationAction.handlePrimaryRequest(TransportReplicationAction.java:377)
    	at org.opensearch.transport.RequestHandlerRegistry.processMessageReceived(RequestHandlerRegistry.java:98)
    	at org.opensearch.transport.TransportService$8.doRun(TransportService.java:937)
    	at org.opensearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:792)
    	at org.opensearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:50)
    	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
    	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)

@nknize
Copy link
Collaborator Author

nknize commented Mar 11, 2022

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 13a592d
Log 3248

Reports 3248

@nknize
Copy link
Collaborator Author

nknize commented Mar 11, 2022

Another #1561 failure; refiring

/gradlew ':server:internalClusterTest' --tests "org.opensearch.cluster.allocation.ClusterRerouteIT.testDelayWithALargeAmountOfShards" -Dtests.seed=7F5EA091AFD9EF57 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=vi -Dtests.timezone=EET -Druntime.java=17
org.opensearch.cluster.allocation.ClusterRerouteIT > testDelayWithALargeAmountOfShards FAILED
    java.lang.AssertionError: AcknowledgedResponse failed - not acked
    Expected: <true>
         but: was <false>
        at __randomizedtesting.SeedInfo.seed([7F5EA091AFD9EF57:532FAA70BEC7C5B5]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked(OpenSearchAssertions.java:128)
        at org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked(OpenSearchAssertions.java:132)
        at org.opensearch.test.TestCluster.wipeIndices(TestCluster.java:167)
        at org.opensearch.test.TestCluster.wipe(TestCluster.java:90)
        at org.opensearch.test.OpenSearchIntegTestCase.afterInternal(OpenSearchIntegTestCase.java:601)
        at org.opensearch.test.OpenSearchIntegTestCase.cleanUpCluster(OpenSearchIntegTestCase.java:2235)

@nknize
Copy link
Collaborator Author

nknize commented Mar 11, 2022

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 13a592d
Log 3249

Reports 3249

@nknize nknize merged commit b00b3ce into opensearch-project:main Mar 11, 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 v2.0.0 Version 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] org.opensearch.upgrades.FullClusterRestartIT > testEmptyShard FAILED
4 participants