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

[da-vinc-client] DVC aggregates push status updates across partitions #1057

Merged

Conversation

huangminchn
Copy link
Contributor

Summary

This PR enables the DVC to write batching push status updates across all partitions on a node.

During batch pushes, one DVC instance will send two events to the push statuses updates to
push status system stores:
one event indicating the push has started in this DVC node;
and another event indicating that the push has completed in all partitions assigned to this DVC node so far.

A config to control this behavior:
write.batching.push.status - by default, it's disabled. Will update default value after enough testing.

The batching only applies to batch push part, it doesn't affect incremental pushes so far.

How was this PR tested?

Added integration tests.

Does this PR introduce any user-facing changes?

  • No. You can skip the rest of this section.
  • Yes. Make sure to explain your proposed changes and call out the behavior change.

@huangminchn huangminchn force-pushed the dvc_write_batch_push_status_update branch 3 times, most recently from c441560 to 81b91fe Compare August 2, 2024 22:55
@huangminchn huangminchn force-pushed the dvc_write_batch_push_status_update branch from 81b91fe to 1a1acf5 Compare August 8, 2024 00:31
Copy link
Contributor

@gaojieliu gaojieliu left a comment

Choose a reason for hiding this comment

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

Left two minor comments.

@huangminchn huangminchn force-pushed the dvc_write_batch_push_status_update branch from 17c04ef to acc4b86 Compare August 29, 2024 18:42
@huangminchn huangminchn force-pushed the dvc_write_batch_push_status_update branch from acc4b86 to 476ced1 Compare August 29, 2024 23:34
Copy link
Contributor

@gaojieliu gaojieliu left a comment

Choose a reason for hiding this comment

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

Thanks!

@huangminchn huangminchn merged commit c8d0531 into linkedin:main Aug 30, 2024
32 checks passed
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.

3 participants