feat(dal,rebaser): store pending dvu values on snapshot #3790
Conversation
lib/dal/src/workspace_snapshot.rs
Outdated
| self.find_existing_dependent_value(value_id).await?; | ||
|
|
||
| if existing_value_id.is_some() { | ||
| return Ok(()); |
There was a problem hiding this comment.
Didn't we want to "touch" the existing node somehow to force conflict resolution? In the case where there's a DVU running that hasn't had the removal of these go through the rebaser, and we modify something (that won't be visible to the running DVU), we don't want to lose the fact that things still need to run. The conflict should always be a "modified a deleted" (or "deleted a modified") one, and the resolution would be to keep the modified version. In the case of a "conflicting modify", we can deduplicate down to one (doesn't really matter which). Same if we detect that two change sets tried to add the same one when neither started from a base that had it.
| EdgeWeight::new(change_set, EdgeWeightKind::new_use())?, | ||
| action_node_index, | ||
| )?; | ||
| for category_node_kind in CategoryNodeKind::iter() { |
There was a problem hiding this comment.
That feels like this will scale better over time
|
bors r+ |
|
bors r- |
|
Canceled. |
zacharyhamm
force-pushed
the
zack/add-dependent-values-to-root
branch
from
90c8298
to
66b1d95
Compare
|
bors r+ |
|
🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
|
bors r- |
|
forgot i wanted to ensure this was backwards compat |
|
Merge conflict. |
zacharyhamm
force-pushed
the
zack/add-dependent-values-to-root
branch
6 times, most recently
from
be48b12
to
f33c19c
Compare
|
bors r+ |
|
🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
3790: feat(dal,rebaser): store pending dvu values on snapshot r=zacharyhamm a=zacharyhamm Instead of sending the set of values to use as the starting point for calculating the dependent value graph in the DependentValuesUpdate job, place those values on the graph off a new category node. Then, when the job executes them, remove them from the graph. Co-authored-by: Zachary Hamm <zack@systeminit.com>
|
bors r- |
|
Canceled. |
Instead of sending the set of values to use as the starting point for calculating the dependent value graph in the DependentValuesUpdate job, place those values on the graph off a new category node. Then, when the job executes them, remove them from the graph.
zacharyhamm
force-pushed
the
zack/add-dependent-values-to-root
branch
from
f33c19c
to
9c5e486
Compare
|
bors r+ |
|
Build succeeded: |
Instead of sending the set of values to use as the starting point for calculating the dependent value graph in the DependentValuesUpdate job, place those values on the graph off a new category node. Then, when the job executes them, remove them from the graph.