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.

Sign up for GitHub

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

Jump to bottom

[Remove] types from translog #2439

Merged
merged 3 commits into from
Mar 12, 2022
Merged

[Remove] types from translog #2439

merged 3 commits into from
Mar 12, 2022

Conversation

nknize
Copy link
Collaborator

@nknize nknize commented Mar 11, 2022

Removes persisting the type in the translog since types are no longer supported.

relates #1940

@nknize nknize requested a review from a team as a code owner March 11, 2022 05:20
@opensearch-ci-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 7bcc741e8d133da786577225829ee89851c3c9b7
Log 3253

Reports 3253

@dreamer-89
Copy link
Member 

REPRODUCE WITH: ./gradlew ':qa:remote-clusters:integTest' --tests "org.opensearch.cluster.remote.test.RemoteClustersIT.testHAProxyModeConnectionWorks" -Dtests.seed=5B2C2A90BDC3D724 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=zh-Hant-HK -Dtests.timezone=Etc/Universal -Druntime.java=17

org.opensearch.cluster.remote.test.RemoteClustersIT > testHAProxyModeConnectionWorks FAILED
    java.lang.AssertionError
        at __randomizedtesting.SeedInfo.seed([5B2C2A90BDC3D724:5C5AFF5B8645A179]:0)
        at org.junit.Assert.fail(Assert.java:87)
        at org.junit.Assert.assertTrue(Assert.java:42)
        at org.junit.Assert.assertTrue(Assert.java:53)
        at org.opensearch.cluster.remote.test.RemoteClustersIT.testHAProxyModeConnectionWorks(RemoteClustersIT.java:125)

@dreamer-89
Copy link
Member

start gradle check

@nknize
Copy link
Collaborator Author

nknize commented Mar 11, 2022

RemoteClustersIT.testHAProxyModeConnectionWorks

#1703?

@nknize
[Remove] types from translog
a141927 
Removes persisting the type in the translog since types are no longer supported.

Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
@dreamer-89
Copy link
Member

RemoteClustersIT.testHAProxyModeConnectionWorks

#1703?

I think issue was previously closed as it wasn't reproduicble when run in isolation. From #1703, I could see gradle check failures are still happening because of this issue. Re-opened the issue to get more traction.

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 7bcc741e8d133da786577225829ee89851c3c9b7
Log 3256

Reports 3256

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure a141927
Log 3257

Reports 3257

@nknize
fix malformed json
ec2ff86 
Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure ec2ff86
Log 3263

Reports 3263

reta
reta reviewed Mar 11, 2022
View reviewed changes
test/framework/src/main/java/org/opensearch/index/engine/TranslogHandler.java
}

private void applyOperation(Engine engine, Engine.Operation operation) throws IOException {
switch (operation.operationType()) {
case INDEX:
Engine.Index engineIndex = (Engine.Index) operation;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am concerned about this change: indeed mapping.merge does not modify the source mapping (at least, mush not), BUT removing mapping.merge(update, MapperService.MergeReason.MAPPING_RECOVERY) may now hide any issues which may have been surfaced during the mapping reconciliation (fe check ObjectMapper::doMerge).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is stale code as of #2077 since translog is no longer used in recovery

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I see, could we remove it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Might look into doing that in another PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Might look into doing that in another PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@tlfeng
Copy link
Collaborator

tlfeng commented Mar 11, 2022 •
edited
Loading

Test failure in Log 3263 is reported in issue #2440, seems a new test failure occurred since yesterday.

@nknize
ignore 404 for now
4577e73 
Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 4577e73
Log 3273

Reports 3273

@nknize
Copy link
Collaborator Author

nknize commented Mar 12, 2022

Another #2442; refiring:

./gradlew ':server:internalClusterTest' --tests "org.opensearch.blocks.SimpleBlocksIT.testAddBlockWhileDeletingIndices" -Dtests.seed=1489AF98B3B5ACD5 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=id -Dtests.timezone=Canada/Mountain -Druntime.java=17

org.opensearch.blocks.SimpleBlocksIT > testAddBlockWhileDeletingIndices FAILED
    com.carrotsearch.randomizedtesting.UncaughtExceptionError: Captured an uncaught exception in thread: Thread[id=721, name=Thread-16, state=RUNNABLE, group=TGRP-SimpleBlocksIT]

        Caused by:
        java.lang.AssertionError: 
        Expected: an instance of org.opensearch.index.IndexNotFoundException
             but: <ProcessClusterEventTimeoutException[failed to process cluster event (delete-index [[oqgjyotdwb/9CKKr4ncQ5ePQyF8JhNQSQ]]) within 30s]> is a org.opensearch.cluster.metadata.ProcessClusterEventTimeoutException
            at __randomizedtesting.SeedInfo.seed([1489AF98B3B5ACD5]:0)
            at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
            at org.junit.Assert.assertThat(Assert.java:964)
            at org.junit.Assert.assertThat(Assert.java:930)
            at org.opensearch.blocks.SimpleBlocksIT.lambda$testAddBlockWhileDeletingIndices$14(SimpleBlocksIT.java:483)
            at org.opensearch.blocks.SimpleBlocksIT.lambda$testAddBlockWhileDeletingIndices$15(SimpleBlocksIT.java:498)

@nknize
Copy link
Collaborator Author

nknize commented Mar 12, 2022

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 4577e73
Log 3276

Reports 3276

@nknize nknize merged commit 95d4750 into opensearch-project:main Mar 12, 2022
@tlfeng tlfeng mentioned this pull request Mar 13, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reta reta reta approved these changes

Assignees
No one assigned
Labels
Indexing & Search v2.0.0 Version 2.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nknize @opensearch-ci-bot @dreamer-89 @tlfeng @reta