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

Some Cleanup in o.e.i.engine #42278

Conversation

original-brownbear
Copy link
Member

  • Remove dead code and parameters
  • Reduce visibility in some obvious spots
  • Add missing asserts (not that important here since the methods
    themselves will probably be dead-code eliminated) but still

* Remove dead code and parameters
* Reduce visibility in some obvious spots
* Add missing `assert`s (not that important here since the methods
themselves will proably be dead-code eliminated) but still
@original-brownbear original-brownbear added >non-issue :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. v8.0.0 v7.3.0 labels May 21, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@original-brownbear
Copy link
Member Author

Jenkins run elasticsearch-ci/2
Jenkins run elasticsearch-ci/bwc

@original-brownbear
Copy link
Member Author

Jenkins run elasticsearch-ci/bwc

Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

I left some minor comments but this refactoring makes sense to me. 👍

return new IndexingStrategy(true, true, true,
false, versionForIndexing, null);
false, (long) 1, null);
Copy link
Contributor

Choose a reason for hiding this comment

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

s/(long) 1/1L/ ?

@@ -457,8 +457,8 @@ public void updateMaxUnsafeAutoIdTimestamp(long newTimestamp) {

}

protected void processReaders(IndexReader reader, IndexReader previousReader) {
searcherFactory.processReaders(reader, previousReader);
protected void processReaders(IndexReader reader) {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe rename the method as well since we have a single reader now?

@@ -59,7 +59,7 @@ public CodecReader wrapForMerge(CodecReader reader) throws IOException {
}

// pkg private for testing
static CodecReader wrapReader(String recoverySourceField, CodecReader reader, Supplier<Query> retainSourceQuerySupplier)
private static CodecReader wrapReader(String recoverySourceField, CodecReader reader, Supplier<Query> retainSourceQuerySupplier)
Copy link
Contributor

Choose a reason for hiding this comment

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

remove the above comment now?

@original-brownbear
Copy link
Member Author

thanks @jpountz !

@original-brownbear original-brownbear merged commit 7ab59ee into elastic:master May 22, 2019
@original-brownbear original-brownbear deleted the fix-internal-engine-asserts branch May 22, 2019 10:55
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request May 27, 2019
* Some Cleanup in o.e.i.engine

* Remove dead code and parameters
* Reduce visibility in some obvious spots
* Add missing `assert`s (not that important here since the methods
themselves will probably be dead-code eliminated) but still
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
* Some Cleanup in o.e.i.engine

* Remove dead code and parameters
* Reduce visibility in some obvious spots
* Add missing `assert`s (not that important here since the methods
themselves will probably be dead-code eliminated) but still
original-brownbear added a commit that referenced this pull request May 27, 2019
* Some Cleanup in o.e.i.engine

* Remove dead code and parameters
* Reduce visibility in some obvious spots
* Add missing `assert`s (not that important here since the methods
themselves will probably be dead-code eliminated) but still
@original-brownbear original-brownbear restored the fix-internal-engine-asserts branch August 6, 2020 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. >non-issue v7.3.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants