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 unnecessary intermediate serialization in RaftLog#append #6671

Closed
npepinpe opened this issue Mar 29, 2021 · 0 comments · Fixed by #10320
Closed

Remove unnecessary intermediate serialization in RaftLog#append #6671

npepinpe opened this issue Mar 29, 2021 · 0 comments · Fixed by #10320
Assignees
Labels
area/performance Marks an issue as performance related kind/toil Categorizes an issue or PR as general maintenance, i.e. cleanup, refactoring, etc. scope/broker Marks an issue or PR to appear in the broker section of the changelog version:8.1.0 Marks an issue as being completely or in parts released in 8.1.0

Comments

@npepinpe
Copy link
Member

Description

At the moment, we first serialize the Raft log entry before passing it to the journal. This works fine, but requires us to have an intermediate in memory buffer. This means we have an extra copy in the journal when appending, and we have this expendable, long living buffer in memory.

Proposal:

What we need is a way to serialize only when we have the final destination buffer. One easy way is to amend the Journal interface to add a public JournalRecord append(int entryLength, Consumer<MutableDirectBuffer> entryWriter);. Or we we want to obviate the need for lambdas:

public interface RecordDataWriter<T> {
  final void write(T entry, MutableDirectBuffer buffer, int offset);
}

public interface Journal {
  // ...
  public <T> JournalRecord append(T entry, int entryLength, RecordDataWriter<T> writer)
}

In the journal, we can then serialize the record data when we need it directly to the writer's buffer instead of using an intermediate write buffer.

NOTE: if we can make it without using generics, even better 🙂

/cc @deepthidevaki

@npepinpe npepinpe added kind/toil Categorizes an issue or PR as general maintenance, i.e. cleanup, refactoring, etc. scope/broker Marks an issue or PR to appear in the broker section of the changelog area/performance Marks an issue as performance related Status: Needs Priority labels Mar 29, 2021
@npepinpe npepinpe added this to the Journal Refactor milestone Mar 29, 2021
@deepthidevaki deepthidevaki removed this from the Journal Refactor milestone Mar 29, 2021
@megglos megglos self-assigned this Sep 13, 2022
zeebe-bors-camunda bot added a commit that referenced this issue Sep 16, 2022
10320: feat(journal): removal of intermediate serialisation in raft append r=megglos a=megglos

## Description

This change removes the intermediate buffer previously used in `RaftLog` to serialise the raft log entry before passing it to the journal where it is written to the journal log. Instead a recordWriter instance is passed to the journal which holds a reference to the entry and directly writes the data to the writeBuffer when the actual journal log entry is written.

## Related issues

<!-- Which issues are closed by this PR or are related -->

closes #6671 



10378: refactor(snapshots): remove unnecessary close in snapshot r=megglos a=deepthidevaki

## Description

`PersistedSnapshot` implemented `CloseableSilently`. This was creating warnings in several places that a snapshot is not used with try-with-resources. But all implementations of `close` did nothing. There is no need to close a snapshot as it is not holding any resources.



Co-authored-by: Meggle (Sebastian Bathke) <sebastian.bathke@camunda.com>
Co-authored-by: Deepthi Devaki Akkoorath <deepthidevaki@gmail.com>
@Zelldon Zelldon added the version:8.1.0 Marks an issue as being completely or in parts released in 8.1.0 label Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/performance Marks an issue as performance related kind/toil Categorizes an issue or PR as general maintenance, i.e. cleanup, refactoring, etc. scope/broker Marks an issue or PR to appear in the broker section of the changelog version:8.1.0 Marks an issue as being completely or in parts released in 8.1.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants