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

Upgrade to Lucene 6.2.0 #20147

Merged
merged 4 commits into from
Aug 25, 2016
Merged

Conversation

mikemccand
Copy link
Contributor

The upgrade was mostly uneventful, except Completion090PostingsFormat had to switch from an ordinary codec header to an index file header (Lucene now requires this of all files that enter a CFS), and
BlobStoreRepository.restoreFile sometimes needs to overwrite an existing file (not sure why, seems dangerous? I left a TODO).

Lucene now requires that all files created with Directory.createOutput are new; I was surprised more places in ES don't seem angry about that.

I also fixed Store.renameTempFilesSafe to only fsync after all files are renamed.

Closes #20092

@mikemccand mikemccand added :Core/Infra/Core Core issues without another label >upgrade v5.0.0-beta1 labels Aug 24, 2016
@mikemccand mikemccand self-assigned this Aug 24, 2016
@@ -299,6 +300,7 @@ public StoreStats stats() throws IOException {

public void renameFile(String from, String to) throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

just remove it, it seems only test use this method now?

@s1monw
Copy link
Contributor

s1monw commented Aug 24, 2016

looks great - left some minors

@mikemccand
Copy link
Contributor Author

I pushed some more commits @s1monw.

@s1monw
Copy link
Contributor

s1monw commented Aug 25, 2016

LGTM

@@ -138,16 +138,6 @@ Similarity getDefaultSimilarity() {
}

@Override
public float coord(int overlap, int maxOverlap) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should change the constructor a few lines above in this file to extend PerFieldSimilarityWrapper(defaultSimilarity) rather than the deprecated default constructor?

@jpountz
Copy link
Contributor

jpountz commented Aug 25, 2016

I left one comment about SimilarityService but otherwise LGTM

@mikemccand
Copy link
Contributor Author

Thanks @jpountz, good catch! I pushed a new commit to call super(baseSimilarity) instead.

@jpountz
Copy link
Contributor

jpountz commented Aug 25, 2016

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >upgrade v5.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to Lucene 6.2.0
5 participants