Skip to content

Enforce access to translog via engine #29542

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

Merged
merged 2 commits into from
Apr 17, 2018
Merged

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Apr 16, 2018

Today the translog of an engine is exposed and can be accessed directly.
While this exposure offers much flexibility, it also causes these troubles:

  • Inconsistent behavior between translog method and engine method.
    For example, rolling a translog generation via an engine also trims
    unreferenced files, but translog's method does not.

  • An engine does not get notified when critical errors happen in its
    translog as the access is direct.

This change isolates translog of an engine and enforces all accesses to
translog via the engine.

Today the translog of an engine is exposed and can be accessed directly.
While this exposure offers much flexibility, it also causes these troubles:

- Inconsistent behavior between translog method and engine method.
For example, rolling a translog generation via an engine also trims
unreferenced files, but translog's method does not.

- An engine does not get notified when critical errors happen in translog
as the access is direct.

This change isolates translog of an engine and enforces all accesses to
translog via the engine.
@dnhatn dnhatn added >enhancement v7.0.0 v6.3.0 :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. labels Apr 16, 2018
@dnhatn dnhatn requested a review from bleskes April 16, 2018 19:14
@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. Thanks @dnhatn

public void setTranslog(Translog translog) {
this.translog = translog;
public void setCurrentRefreshLocationSupplier(Supplier<Translog.Location> currentRefreshLocationSupplier) {
this.currentRefreshLocationSupplier = currentRefreshLocationSupplier;
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

@dnhatn
Copy link
Member Author

dnhatn commented Apr 17, 2018

Thanks @bleskes.

@dnhatn dnhatn merged commit 45c6c20 into elastic:master Apr 17, 2018
@dnhatn dnhatn deleted the isolate-translog branch April 17, 2018 12:03
dnhatn added a commit that referenced this pull request Apr 18, 2018
Today the translog of an engine is exposed and can be accessed directly.
While this exposure offers much flexibility, it also causes these troubles:

- Inconsistent behavior between translog method and engine method.
For example, rolling a translog generation via an engine also trims
unreferenced files, but translog's method does not.

- An engine does not get notified when critical errors happen in translog
as the access is direct.

This change isolates translog of an engine and enforces all accesses to
translog via the engine.
dnhatn added a commit that referenced this pull request Apr 18, 2018
* master:
  Remove the index thread pool (#29556)
  Remove extra copy in ScriptDocValues.Strings
  Fix full cluster restart test recovery (#29545)
  Fix binary doc values fetching in _search (#29567)
  Mutes failing MovAvgIT tests
  Fix the assertion message for an incorrect current version. (#29572)
  Fix the version ID for v5.6.10. (#29570)
  Painless Spec Documentation Clean Up (#29441)
  Add versions 5.6.10 and 6.2.5
  [TEST] test against scaled value instead of fixed epsilon in MovAvgIT
  Remove `flatSettings` support from request classes (#29560)
  MapperService to wrap a single DocumentMapper. (#29511)
  Fix dependency checks on libs when generating Eclipse configuration. (#29550)
  Add null_value support to geo_point type (#29451)
  Add documentation about the include_type_name option. (#29555)
  Enforce translog access via engine (#29542)
martijnvg pushed a commit that referenced this pull request Apr 25, 2018
Since #29542, we no longer expose translog instance
but only provide creating translog snapshot method. This commit adapts
that change in CCR branch.

Relates #29542
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.3.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants