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

Do not store intermediate commits during merge/transplant #6677

Merged
merged 1 commit into from
May 2, 2023

Conversation

dimas-b
Copy link
Member

@dimas-b dimas-b commented Apr 24, 2023

  • Do not store objects during dry runs.

  • Do not store objects when conflicts are detected.

Use a BatchingPersist during unsquashed merge/transplant execution and
flush only on success. This is to allow CommitLogic to 'see'
intermediate commits without writing them to storage. Batching writes in
this case is a secondary effect.

Move PersistDelegate to top level.

Add test tooling to allow validating whether any storage writes happened
since a checkpoint.

Also add a test case to make sure dry run merge/transplant operations
at the API level.

Note: exact response messages differ between old and new model in
this case, but substantial information is the same (keys from the
merge/transplant request).

Fixes #6674

@dimas-b dimas-b requested a review from snazy April 24, 2023 18:34
adutra
adutra previously approved these changes Apr 25, 2023
snazy
snazy previously approved these changes Apr 28, 2023
@dimas-b
Copy link
Member Author

dimas-b commented Apr 28, 2023

I going to push some significant changes (after resolving conflicts). Adding test utilities for validating storage writes (or absence of them).

* Do not store objects during dry runs.

* Do not store objects when conflicts are detected.

Use a BatchingPersist during unsquashed merge/transplant execution and
flush only on success. This is to allow CommitLogic to 'see'
intermediate commits without writing them to storage. Batching writes in
this case is a secondary effect.

Move PersistDelegate to top level.

Add test tooling to allow validating whether any storage writes happened
since a checkpoint.

Also add a test case to make sure dry run merge/transplant operations
at the API level.

Note: exact response messages differ between old and new model in
this case, but substantial information is the same (keys from the
merge/transplant request).

Fixes projectnessie#6674
@dimas-b dimas-b changed the title Do not store intermediate commits during dry run merge/transplant Do not store intermediate commits during merge/transplant Apr 28, 2023
@dimas-b dimas-b marked this pull request as ready for review April 28, 2023 21:04
@dimas-b dimas-b requested review from adutra and snazy April 28, 2023 21:04
@dimas-b dimas-b merged commit 7ee678c into projectnessie:main May 2, 2023
24 checks passed
@dimas-b dimas-b deleted the fix-dry-run-merge branch May 2, 2023 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dry-run Merge still makes writes in the New Model
3 participants