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

LUCENE-9047: Write checksum as big endian in NRT replicator #123

Merged
merged 2 commits into from
May 3, 2021

Conversation

iverase
Copy link
Contributor

@iverase iverase commented May 3, 2021

Left over from #107, we should read / write checksums using big endian in the NRT replicator.

@iverase iverase changed the title LUCENE-9047: Write checksum as Big endian in NRT replicator LUCENE-9047: Write checksum as big endian in NRT replicator May 3, 2021
@iverase iverase merged commit a91bde5 into apache:main May 3, 2021
@iverase iverase deleted the fixnrt branch May 3, 2021 07:29
@uschindler
Copy link
Contributor

uschindler commented May 3, 2021

Hi @iverase
the merge broke tests: We fixed over the weekend the NRT tests. They had a bug in the security policy, so no NRT Test wasn't running at all. So you did not see the test failure.

If you fix it, make sure to run the replicator tests with -Dtests.nightly=true, so it runs all tests (takes a few minutes)

I will backport the test fixes also to 8.x later. It's great that we fixed it yesterday :-) The NRT tests were not running since now YEARS!

@iverase
Copy link
Contributor Author

iverase commented May 3, 2021

Thanks @uschindler, yikes that explain it all :)

This fix seems to work, I run the test with the nightly flag and it seems happy:

 ./gradlew :lucene:replicator:test -Dtests.nightly=true

> Task :randomizationInfo
Running tests with randomization seed: tests.seed=3735F7B6C10CE977

> Task :lucene:replicator:test
:lucene:replicator:test (SUCCESS): 44 test(s), 1 skipped
The slowest tests (exceeding 500 ms) during this run:
  190.02s TestStressNRTReplication.test (:lucene:replicator)
   7.52s TestIndexAndTaxonomyReplicationClient.testConsistencyOnExceptions (:lucene:replicator)
   6.52s TestNRTReplication.testFullClusterCrash (:lucene:replicator)
   5.44s TestIndexReplicationClient.testConsistencyOnExceptions (:lucene:replicator)
   3.49s TestNRTReplication.testCrashPrimary3 (:lucene:replicator)
   3.36s TestNRTReplication.testCrashReplica (:lucene:replicator)
   3.27s TestNRTReplication.testCrashPrimary2 (:lucene:replicator)
   2.65s TestNRTReplication.testReplicaCrashWithCommit (:lucene:replicator)
   2.52s TestNRTReplication.testCrashPrimary1 (:lucene:replicator)
   2.52s TestNRTReplication.testIndexingWhileReplicaIsDown (:lucene:replicator)
The slowest suites (exceeding 1s) during this run:
  190.06s TestStressNRTReplication (:lucene:replicator)
  32.55s TestNRTReplication (:lucene:replicator)
   8.33s TestIndexAndTaxonomyReplicationClient (:lucene:replicator)
   6.28s TestIndexReplicationClient (:lucene:replicator)

BUILD SUCCESSFUL in 3m 27s

@uschindler
Copy link
Contributor

What did you change, or was the test failures on Jenkins only because of the first merge on the directory API and this one fixed it?

This explains why you opened THIS pull request. Sorry for the noise.

Uwe

@uschindler
Copy link
Contributor

It will be funny, when Solr picks the new snapshot builds!

@mikemccand
Copy link
Member

The NRT tests were not running since now YEARS!

OH NO!

@uschindler
Copy link
Contributor

I backported the fixes. But I can tell you: It was only a master problm, 8.x was fine. I still backported Dawid's fixes.

@uschindler
Copy link
Contributor

the suppressAccessChecks permission got lost in master, so the NRT tests were not able to crush the test server with SIGSEGV. It's now working in both master and 8.x. I ran the tests and all worked.

@mikemccand
Copy link
Member

Awesome, thank you @uschindler!

mikemccand pushed a commit to mikemccand/lucene that referenced this pull request Sep 3, 2021
…cValues=true" a.k.a. pure DocValues fields. (apache#123)

(Stanislav Livotov, Erick Erickson, Tobias Kässmann, Tom Gilke, Christine Poerschke)
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.

3 participants