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-vinci][controller] Filter out status reporting from the nodes, where the DaVinci app is still bootstrapping #1203

Conversation

gaojieliu
Copy link
Contributor

@gaojieliu gaojieliu commented Sep 27, 2024

Summary

In Prod, we are seeing the bootstrapping nodes are blocking the new push, and this PR will let Controller ignore the status report from the nodes, which are still bootstrapping.
This feature is especially important for DaVinci apps, which are using multiple large stores and bootstrapping from an empty state can take a long time and it can delay the new push significantly.

How was this PR tested?

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.

FelixGV
FelixGV previously approved these changes Sep 27, 2024
Copy link
Contributor

@FelixGV FelixGV left a comment

Choose a reason for hiding this comment

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

Thanks, makes sense, LGTM.

@gaojieliu gaojieliu changed the title [da-vinci] Disabled status reporting when the DaVinci app is still bootstrapping [da-vinci] Filter out status reporting from the nodes, where the DaVinci app is still bootstrapping Sep 30, 2024
@gaojieliu gaojieliu changed the title [da-vinci] Filter out status reporting from the nodes, where the DaVinci app is still bootstrapping [da-vinci][Controller] Filter out status reporting from the nodes, where the DaVinci app is still bootstrapping Oct 1, 2024
@gaojieliu gaojieliu changed the title [da-vinci][Controller] Filter out status reporting from the nodes, where the DaVinci app is still bootstrapping [da-vinci][controller] Filter out status reporting from the nodes, where the DaVinci app is still bootstrapping Oct 1, 2024
…otstrapping

In Prod, we are seeing the bootstrapping nodes are blocking the
new push, and this PR disables the status reporting if the app
is still bootstrapping.
This feature is especially important for DaVinci apps, which are
using multiple large stores and bootstrapping from an empty state
can take a long time and it can delay the new push significantly.
1. Subscription won't return if there are still active current version bootstrapping
   to avoid serving stale current version.
2. Delete the heartbeat entry for the current instance if the instance is bootstrapping
   to avoid Controller mark the bootstrapping node as crashed, which can fail the
   new push jobs.
@gaojieliu gaojieliu force-pushed the bootstrapping_dvc_not_report_new_version_status branch from 57e2d9d to 4750def Compare October 1, 2024 16:29
Copy link
Contributor

@sixpluszero sixpluszero left a comment

Choose a reason for hiding this comment

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

Thanks for the change and offline discussion!

@gaojieliu gaojieliu merged commit b9685bb into linkedin:main Oct 2, 2024
45 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