Make modifying operations durable by default. #11011
Conversation
| */ | ||
| public void setDurable(boolean durable) { | ||
| this.durable = durable; | ||
| } |
There was a problem hiding this comment.
existing getters in this class use the non getter getters convention and settters return this. Don't want to start the whole discussion here but maybe at least in the same class we should follow one single convention.
There was a problem hiding this comment.
setters are the way to go I will fix all the others but not in this PR
There was a problem hiding this comment.
you mean you will fix every getter and setter in the java api? I didnt know we finally made a decision around this... would love to have a single way to do things throughout the codebase though
There was a problem hiding this comment.
yeah we should just do what every java project does and use setters and getters
|
@rmuir I pushed a new commit and replied to your comment |
|
@s1monw +1, looks great. As a follow up to this change, we need to think about an index level setting for it (or maybe adapt the "sync" config to do this automatically when set to 0), and have more benchmarks on more adventurous environments so we can help explain the tradeoffs |
|
@kimchy after thinking about this I think we should go only with an index level setting and drop the entire per request thing. The reason is that with per-request settings you need to change an application to change the value while with index level settings we can do this via the API which is more flexible and safer, opinions? |
|
@s1monw that was my reasoning towards having an index level setting for it, I tend to prefer it, we can always add per request later if needed |
|
ok moving towards index level setting |
|
thanks for improving the loop with asserts/comments! |
|
@kimchy I think it's ready now with an index level setting |
|
@s1monw this looks great |
This commit makes create, update and delete operations on an index durable by default. The user has the option to opt out to use async translog flushes on a per-index basis by settings `index.translog.durability=request`. Initial benchmarks running on SSDs have show that indexing is about 7% - 10% slower with bulk indexing compared to async translog flushes. This change is orthogonal to the transaction log sync interval and will only sync the transaction log if the operation has not yet been concurrently synced. Ie. if multiple indexing requests are submitted and one operations sync call already persists the operations of others only one sync call is executed. Relates to elastic#10933
|
Should we document this new setting and add a note to the resiliency status page? |
|
@jpountz I don't think we are done yet. I want to open a documentation issue once we are setted with all thte things - the more important issue where we fix the translog corruption is still coming |
|
Sounds great. Just wanted to make sure it doesn't get forgotten. :) |
Today we are almost intentionally corrupt the translog if we loose a node due to powerloss or similary disasters. In the translog reading code we simply read until we hit an EOF exception ignoring the rest of the translog file once hit. There is no information stored how many records we are expecting or what the last written offset was. This commit restructures the translog to add checkpoints that are written with every sync operation recording the number of synced operations as well as the last synced offset. These checkpoints are also used to identify the actual transaction log file to open instead of relying on directory traversal. This change adds a significant amount of additional checks and pickyness to the translog code. For instance is the translog now associated with a specific engine via a UUID that is written to each translog file as part of it's header. If an engine opens a translog file it was not associated with the operation will fail. Closes to elastic#10933 Relates to elastic#11011
clintongormley
added
:Distributed Indexing/Distributed
This commit makes create, update and delete operations on an index durable
by default. The user has the option to opt out to use async translog flushes
on a per-index basis by settings
index.translog.durability=request.Initial benchmarks running on SSDs have show that indexing is about 7% - 10% slower
with bulk indexing compared to async translog flushes. This change is orthogonal to
the transaction log sync interval and will only sync the transaction log if the operation
has not yet been concurrently synced. Ie. if multiple indexing requests are submitted and
one operations sync call already persists the operations of others only one sync call is executed.
Relates to #10933