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

[Upgrade] to lucene-9.4.0-snapshot-ddf0d0a #4183

Merged
merged 3 commits into from
Aug 15, 2022
Merged

[Upgrade] to lucene-9.4.0-snapshot-ddf0d0a #4183

merged 3 commits into from
Aug 15, 2022

Conversation

nknize
Copy link
Collaborator

@nknize nknize commented Aug 10, 2022

Upgrades to latest snapshot of lucene 9.4.

@nknize nknize added non-issue bugs / unexpected behaviors that end up non issues; audit trail simple changes that aren't issues >upgrade Label used when upgrading library dependencies (e.g., Lucene) v3.0.0 Issues and PRs related to version 3.0.0 labels Aug 10, 2022
@nknize nknize requested review from a team and reta as code owners August 10, 2022 14:51
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@nknize nknize marked this pull request as draft August 10, 2022 18:31
navneet1v
navneet1v reviewed Aug 11, 2022
View reviewed changes
.../main/java/org/opensearch/index/fielddata/fieldcomparator/BytesRefFieldComparatorSource.java
Comment on lines -125 to -128
@Override
public void setScorer(Scorable scorer) {
BytesRefFieldComparatorSource.this.setScorer(scorer);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure what was the use of this and when we are removing it will this impact anything.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It doesn't impact anything since the setScorer method is a noop

@nknize nknize force-pushed the upgrade/lucene9.4-snapshot-ddf0d0a branch from 2233fa2 to e456618 Compare August 12, 2022 20:44
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

nknize added 3 commits August 15, 2022 11:56
@nknize
[Upgrade] to lucene-9.4.0-snapshot-ddf0d0a
86c95e4 
Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
@nknize
add fieldname to BytesRefFieldComparatorSource
02ce7aa 
Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
@nknize
set to not skip docs
38c237c 
Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
@nknize nknize force-pushed the upgrade/lucene9.4-snapshot-ddf0d0a branch from e456618 to 38c237c Compare August 15, 2022 16:56
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@nknize
Copy link
Collaborator Author

nknize commented Aug 15, 2022

Hmm.. can't repro locally. Refiring and will continue to dig...

REPRODUCE WITH: ./gradlew ':server:test' --tests "org.opensearch.index.fielddata.SortedSetDVStringFieldDataTests.testSortMissingFirst" -Dtests.seed=3AA5F19D0B1A959 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ar -Dtests.timezone=America/Mexico_City -Druntime.java=17
NOTE: leaving temporary files on disk at: /var/jenkins/workspace/gradle-check/search/server/build/testrun/test/temp/org.opensearch.index.fielddata.SortedSetDVStringFieldDataTests_3AA5F19D0B1A959-001

java.lang.AssertionError: expected:<1055> but was:<1053>
	at __randomizedtesting.SeedInfo.seed([3AA5F19D0B1A959:90B5BA3675D39746]:0)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at org.junit.Assert.assertEquals(Assert.java:633)
	at org.opensearch.index.fielddata.AbstractStringFieldDataTestCase.testSortMissing(AbstractStringFieldDataTestCase.java:343)
	at org.opensearch.index.fielddata.AbstractStringFieldDataTestCase.testSortMissingFirst(AbstractStringFieldDataTestCase.java:301)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)

@nknize
Copy link
Collaborator Author

nknize commented Aug 15, 2022

start gradle check

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@nknize nknize marked this pull request as ready for review August 15, 2022 19:46
@nknize nknize merged commit 411321e into opensearch-project:main Aug 15, 2022
@reta reta mentioned this pull request Aug 16, 2022
Merged
5 tasks
@kartg kartg mentioned this pull request Aug 16, 2022
Closed
dbwiddis
dbwiddis reviewed Aug 19, 2022
View reviewed changes
.../main/java/org/opensearch/index/fielddata/fieldcomparator/BytesRefFieldComparatorSource.java
@@ -99,7 +100,7 @@ public FieldComparator<?> newComparator(String fieldname, int numHits, boolean e
final boolean sortMissingLast = sortMissingLast(missingValue) ^ reversed;
final BytesRef missingBytes = (BytesRef) missingObject(missingValue, reversed);
if (indexFieldData instanceof IndexOrdinalsFieldData) {
return new FieldComparator.TermOrdValComparator(numHits, null, sortMissingLast) {
FieldComparator<?> cmp = new TermOrdValComparator(numHits, indexFieldData.getFieldName(), sortMissingLast, reversed) {
Copy link
Member

Choose a reason for hiding this comment

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

@nknize This may have caused a regression, see https://build.ci.opensearch.org/job/gradle-check/1941/

Copy link
Member

Choose a reason for hiding this comment

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

(Also in https://build.ci.opensearch.org/job/gradle-check/1752/ associated with this PR)

Copy link
Member

Choose a reason for hiding this comment

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

Disregard the above. Found the problem elsewhere.

@dbwiddis dbwiddis mentioned this pull request Aug 19, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbwiddis dbwiddis dbwiddis left review comments

@navneet1v navneet1v navneet1v left review comments

@reta reta reta approved these changes

Assignees
No one assigned
Labels
non-issue bugs / unexpected behaviors that end up non issues; audit trail simple changes that aren't issues >upgrade Label used when upgrading library dependencies (e.g., Lucene) v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nknize @reta @navneet1v @dbwiddis