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

Fold EngineSearcher into Engine.Searcher #34082

Merged
merged 5 commits into from
Sep 27, 2018

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Sep 26, 2018

EngineSearcher can be easily folded into Engine.Searcher which removes
a level of inheritance that is necessary for most of it's subclasses.
This change folds it into Engine.Searcher and removes the dependency on
ReferenceManager.

EngineSearcher can be easily folded into Engine.Searcher which removes
a level of inheritance that is necessary for most of it's subclasses.
This change folds it into Engine.Searcher and removes the dependency on
ReferenceManager.
@s1monw s1monw added >enhancement v7.0.0 :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. v6.5.0 labels Sep 26, 2018
@s1monw s1monw requested a review from jasontedor September 26, 2018 14:03
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

Copy link
Contributor

@bleskes bleskes left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

:trollface:

return searcher;
}

@Override
public void close() {
// Nothing to close here
if (!released.compareAndSet(false, true)) {
Copy link
Member

Choose a reason for hiding this comment

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

OMG == false! 😱

Copy link
Member

Choose a reason for hiding this comment

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

You can push a fix for this oversight directly, dismiss my disapproving review, and merge. 😇

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pushed a commit with an appropriate commit message

@s1monw s1monw merged commit bda7bc1 into elastic:master Sep 27, 2018
@s1monw s1monw deleted the simplify_engine_searcher branch September 27, 2018 07:06
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Sep 27, 2018
* master: (25 commits)
  [DOCS] Synchronize location of Breaking Changes (elastic#33588)
  [DOCS] Synchronizes captialization in top-level titles (elastic#33605)
  [SQL] Clean up LogicalPlanBuilder#doJoin (elastic#34048)
  Fix remote cluster seeds fallback (elastic#34090)
  [ML][HLRC] Replace REST-based ML test cleanup with the ML client (elastic#34109)
  Handle MatchNoDocsQuery in span query wrappers (elastic#34106)
  Update MovAvgIT AwaitsFix bug url
  Bad regex in CORS settings should throw a nicer error (elastic#34035)
  [HLRC] Support for role mapper expression dsl (elastic#33745)
  Watcher: Reduce script cache churn by checking for mustache tags (elastic#33978)
  Fold EngineSearcher into Engine.Searcher (elastic#34082)
  Mute SpanMultiTermQueryBuilderTests#testToQuery
  TESTS: Enable DEBUG Logging in Flaky Test (elastic#34091)
  TEST: Add engine is closed as expected failure msg
  Adjust bwc version for max_seq_no_of_updates
  Build DocStats from SegmentInfos in ReadOnlyEngine (elastic#34079)
  When creating wildcard queries, use MatchNoDocsQuery when the field type doesn't exist. (elastic#34093)
  [DOCS] Moves graph to docs folder (elastic#33472)
  Mute MovAvgIT#testHoltWintersNotEnoughData
  Security: use default scroll keepalive (elastic#33639)
  ...
kcm pushed a commit that referenced this pull request Oct 30, 2018
EngineSearcher can be easily folded into Engine.Searcher which removes
a level of inheritance that is necessary for most of it's subclasses.
This change folds it into Engine.Searcher and removes the dependency on
ReferenceManager.
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. >enhancement v6.5.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants