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

Improve the traversing of snapshot files #12575

Closed
aivinog1 opened this issue Apr 27, 2023 · 2 comments · Fixed by #12576
Closed

Improve the traversing of snapshot files #12575

aivinog1 opened this issue Apr 27, 2023 · 2 comments · Fixed by #12576
Labels
kind/feature Categorizes an issue or PR as a feature, i.e. new behavior version:8.1.13 Marks an issue as being completely or in parts released in 8.1.13 version:8.2.5 Marks an issue as being completely or in parts released in 8.2.5 version:8.3.0-alpha2 Marks an issue as being completely or in parts released in 8.3.0-alpha2 version:8.3.0 Marks an issue as being completely or in parts released in 8.3.0

Comments

@aivinog1
Copy link
Contributor

Is your feature request related to a problem? Please describe.

I think that it is not a good idea to collect snapshot files in the list.
Check it here:

final SfvChecksum sfvChecksum = createCombinedChecksum(fileStream.toList());
.
Because, after a while, the amount of snapshot files should grow. So, we could instead call the .forEachOrdered method to calculate the snapshot. I will provide the PR soon to see this in action and benchmark this.

Describe the solution you'd like
We should call Stream.forEachOrdered instead of collecting snapshots Files in the list.

Describe alternatives you've considered
We could use Stream.forEach but as I can understand the order is important, so we shouldn't do this.

@aivinog1 aivinog1 added the kind/feature Categorizes an issue or PR as a feature, i.e. new behavior label Apr 27, 2023
@aivinog1
Copy link
Contributor Author

aivinog1 commented May 4, 2023

@Zelldon Hello 👋
I think that this could be addressed to the ZDP Team (sorry if I am wrong 😅)
Could we assign this to someone, please? In our load test environment, we see a little degradation in creating snapshots and we are suspecting this part of the code.
Thank you in advance 🙂

@Zelldon
Copy link
Member

Zelldon commented May 4, 2023

Looks like @deepthidevaki will look into your PR, thanks for providing it :)

zeebe-bors-camunda bot added a commit that referenced this issue May 4, 2023
12576: refactor(snapshots): Replace `Stream.toList` and the for each cycle to `Stream.forEachOrdered` r=deepthidevaki a=aivinog1

## Description

<!-- Please explain the changes you made here. -->
I replaced the collection with a list and the for each cycle to the `Stream.forEactOrdered` call. It should reduce memory footprint and possibly improve latency, especially on a large number of snapshot files.

## Related issues

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

closes #12575 



Co-authored-by: Alexey Vinogradov <vinogradov.a.i.93@gmail.com>
zeebe-bors-camunda bot added a commit that referenced this issue May 4, 2023
12666: [Backport stable/8.2] refactor(snapshots): Replace `Stream.toList` and the for each cycle to `Stream.forEachOrdered` r=deepthidevaki a=backport-action

# Description
Backport of #12576 to `stable/8.2`.

relates to #12575

Co-authored-by: Alexey Vinogradov <vinogradov.a.i.93@gmail.com>
zeebe-bors-camunda bot added a commit that referenced this issue May 4, 2023
12662: [Backport stable/8.1] fix: do not retake backup if it already exists r=deepthidevaki a=deepthidevaki

Backport of #12626 to stable/8.1.

relates to #12623

12665: [Backport stable/8.1] refactor(snapshots): Replace `Stream.toList` and the for each cycle to `Stream.forEachOrdered` r=deepthidevaki a=backport-action

# Description
Backport of #12576 to `stable/8.1`.

relates to #12575

Co-authored-by: Deepthi Devaki Akkoorath <deepthidevaki@gmail.com>
Co-authored-by: Deepthi Devaki Akkoorath <deepthidevaki@users.noreply.github.com>
Co-authored-by: Alexey Vinogradov <vinogradov.a.i.93@gmail.com>
@oleschoenburg oleschoenburg added the version:8.2.5 Marks an issue as being completely or in parts released in 8.2.5 label May 16, 2023
@oleschoenburg oleschoenburg added version:8.3.0-alpha2 Marks an issue as being completely or in parts released in 8.3.0-alpha2 version:8.1.13 Marks an issue as being completely or in parts released in 8.1.13 labels Jun 7, 2023
@megglos megglos added the version:8.3.0 Marks an issue as being completely or in parts released in 8.3.0 label Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes an issue or PR as a feature, i.e. new behavior version:8.1.13 Marks an issue as being completely or in parts released in 8.1.13 version:8.2.5 Marks an issue as being completely or in parts released in 8.2.5 version:8.3.0-alpha2 Marks an issue as being completely or in parts released in 8.3.0-alpha2 version:8.3.0 Marks an issue as being completely or in parts released in 8.3.0
Projects
None yet
4 participants