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

Simplify the Translog constructor by always expecting an existing translog #28676

Merged
merged 10 commits into from
Feb 15, 2018

Conversation

bleskes
Copy link
Contributor

@bleskes bleskes commented Feb 14, 2018

Currently the Translog constructor is capable both of opening an existing translog and creating a new one (deleting existing files). This PR separates these two into separate code paths. The constructors opens files and a dedicated static methods creates an empty translog.

This PR is part of a bigger refactoring in #28245 , so please ignore any ugliness in the Engine code.

@bleskes bleskes added >non-issue v7.0.0 v6.3.0 :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. labels Feb 14, 2018
Copy link
Member

@dnhatn dnhatn left a comment

Choose a reason for hiding this comment

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

LGTM.

- match: { indices.test.primaries.translog.operations: 1 }
- gt: { indices.test.primaries.translog.uncommitted_size_in_bytes: $empty_size }
# we can't check this yet as creation size will contain two empty translog generations. A single
# non empty generation with one op may be smaller or larger than that.
Copy link
Member

Choose a reason for hiding this comment

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

nit: indentation

@bleskes bleskes merged commit beb55d1 into elastic:master Feb 15, 2018
@bleskes bleskes deleted the translog_simplify_open branch February 15, 2018 08:24
bleskes added a commit that referenced this pull request Feb 15, 2018
…nslog (#28676)

Currently the Translog constructor is capable both of opening an existing translog and creating a
new one (deleting existing files). This PR separates these two into separate code paths. The
constructors opens files and a dedicated static methods creates an empty translog.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Feb 15, 2018
* master:
  Add a note to the docs that _cat api `help` option cannot be used if an optional url param is used (elastic#28686)
  Lift error finding utility to exceptions helpers
  Change "tweet" type to "_doc" (elastic#28690)
  [Docs] Add missing word in nested.asciidoc (elastic#28507)
  Simplify the Translog constructor by always expecting an existing translog (elastic#28676)
  Upgrade t-digest to 3.2 (elastic#28295) (elastic#28305)
  Add comment explaining lazy declared versions
dnhatn added a commit that referenced this pull request Mar 17, 2018
A new engine now can have more than one empty translog since #28676.
This cause #testShouldPeriodicallyFlush failed because in the test we
asssume an engine should have one empty translog. This commit takes into
account the extra translog size of a new engine.
dnhatn added a commit that referenced this pull request Mar 17, 2018
A new engine now can have more than one empty translog since #28676.
This cause #testShouldPeriodicallyFlush failed because in the test we
asssume an engine should have one empty translog. This commit takes into
account the extra translog size of a new engine.
dnhatn added a commit that referenced this pull request Mar 31, 2018
If a primary executing the only indexing request is on a 6.2 node, the
translog uncommitted size will equal to the translog creation size
because there is no overhead (eg. multiple empty translog generations)
on the old node. Moreover, since other primaries on current nodes did
not receive writes, the flush request is no-op on these shards, translog
uncommitted size does not change. In this case, the total translog
uncommitted size on all primaries equal to the translog creation size.

This commit loosens the assertion from less-than to less-than-or-equal
to cover this case.

Relates #28676
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Engine Anything around managing Lucene and the Translog in an open shard. >non-issue v6.3.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants