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

Remove allocations and copying from writing translog ops to the network #95048

Conversation

original-brownbear
Copy link
Member

Found this while going through some of the translog code, this should soften the heap blow from very large documents a little ...

There's no reason to prefix ops with their size over the network. We can verify the checksum once after reading each op and just streaming write ops.

There's no reason to prefix ops with their size over the network.
We can verify the checksum once after reading each op and just streaming
write ops.
@original-brownbear original-brownbear added >non-issue :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. v8.8.0 labels Apr 5, 2023
@elasticsearchmachine elasticsearchmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Apr 5, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@@ -1159,9 +1158,6 @@ private Index(final StreamInput in) throws IOException {
}
source = in.readBytesReference();
routing = in.readOptionalString();
if (format < FORMAT_NO_PARENT) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Removing this old dead code here since it's not worth a separate PR, we never set the format variable to something before the no parent format these days.

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

Neat, LGTM. We originally added these length-prefixes in reaction to some Jepsen failures in 1.x (#10933 (comment)) when I suspect this code was used to write ops to disk as well as over the wire.

@original-brownbear
Copy link
Member Author

Thanks David!

@original-brownbear original-brownbear merged commit 7a83e0c into elastic:main Apr 5, 2023
@original-brownbear original-brownbear deleted the save-alloc-translog-message branch April 5, 2023 19:20
@original-brownbear original-brownbear restored the save-alloc-translog-message branch April 18, 2023 20:58
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 Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants