Skip to content

Commit fceb26a

Browse files
data store: wipe workflow data on reload
* When a workflow is reloaded or restarted (node restart infers reload), there may be objects left behind in the store which have been wiped out in the scheduler by the configuration change. * To handle this, we must wipe all objects stored on the workflow (e.g. TaskProxies, TaskDefs, Jobs, etc), and rebuild from scratch. * Note, we don't need to wipe the data on the Workflow object itself (e.g. port, stateTotals, etc), these things will be refreshed by this or subsequent deltas.
1 parent 42ce1c1 commit fceb26a

File tree

9 files changed

+109
-6
lines changed

9 files changed

+109
-6
lines changed

src/services/treeCallback.js

+20-6
Original file line numberDiff line numberDiff line change
@@ -25,27 +25,41 @@ class CylcTreeCallback extends DeltasCallback {
2525
}
2626
}
2727

28-
tearDown (store, errors) {
29-
// never tear down, this callback lives for the live of the UI
28+
before (deltas, store, errors) {
29+
// Wipe all child nodes from a workflow in the data store if a reloaded
30+
// delta is received. Reloaded deltas are sent whenever a workflow is
31+
// restarted or reloaded (note, restarting a workflow implicitly reloads
32+
// it).
33+
//
34+
// When a workflow reloads it is hard to generate the relevant pruned
35+
// and updated deltas to remove any objects which have been wiped out by
36+
// the configuration change, so the easiest solution is to wipe the
37+
// entire tree under the workflow and rebuild from scratch. If we don't
38+
// do this, we can end up with nodes in the store which aren't meant to be
39+
// there and won't get pruned.
40+
if (deltas.updated?.workflow?.reloaded) {
41+
store.commit('workflows/REMOVE_CHILDREN', (deltas.updated.workflow.id))
42+
}
43+
if (deltas.added?.workflow?.reloaded) {
44+
store.commit('workflows/REMOVE_CHILDREN', (deltas.added.workflow.id))
45+
}
3046
}
3147

3248
onAdded (added, store, errors) {
33-
// console.log('ADDED', added)
3449
store.commit('workflows/UPDATE_DELTAS', added)
3550
}
3651

3752
onUpdated (updated, store, errors) {
38-
// console.log('UPDATED', updated)
3953
store.commit('workflows/UPDATE_DELTAS', updated)
4054
}
4155

4256
onPruned (pruned, store, errors) {
4357
store.commit('workflows/REMOVE_DELTAS', pruned)
4458
}
4559

46-
// this callback does not need the before and commit methods
47-
before (a, b, c) {}
60+
// this callback does not need the tearDown and commit methods
4861
commit (a, b, c) {}
62+
tearDown (s, e) {}
4963
}
5064

5165
export default CylcTreeCallback

src/services/workflow.service.js

+1
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,7 @@ class WorkflowService {
329329
const errors = []
330330

331331
// run the global callback first
332+
globalCallback.before(deltas, store, errors)
332333
globalCallback.onAdded(added, store, errors)
333334
globalCallback.onUpdated(updated, store, errors)
334335
globalCallback.onPruned(pruned, store, errors)

src/views/Dashboard.vue

+2
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,8 @@ fragment UpdatedDelta on Updated {
176176
}
177177

178178
fragment WorkflowData on Workflow {
179+
# NOTE: do not request the "reloaded" event here
180+
# (it would cause a race condition with the workflow subscription)
179181
id
180182
status
181183
}

src/views/Graph.vue

+11
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,11 @@ subscription Workflow ($workflowId: ID) {
135135
}
136136
}
137137

138+
fragment WorkflowData on Workflow {
139+
id
140+
reloaded
141+
}
142+
138143
fragment EdgeData on Edge {
139144
id
140145
source
@@ -162,6 +167,9 @@ fragment JobData on Job {
162167
}
163168

164169
fragment AddedDelta on Added {
170+
workflow {
171+
...WorkflowData
172+
}
165173
edges {
166174
...EdgeData
167175
}
@@ -174,6 +182,9 @@ fragment AddedDelta on Added {
174182
}
175183

176184
fragment UpdatedDelta on Updated {
185+
workflow {
186+
...WorkflowData
187+
}
177188
edges {
178189
...EdgeData
179190
}

src/views/SimpleTree.vue

+14
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ subscription SimpleTreeSubscription ($workflowId: ID) {
131131
}
132132

133133
fragment AddedDelta on Added {
134+
workflow {
135+
...WorkflowData
136+
}
134137
taskProxies {
135138
...TaskProxyData
136139
}
@@ -140,6 +143,9 @@ fragment AddedDelta on Added {
140143
}
141144

142145
fragment UpdatedDelta on Updated {
146+
workflow {
147+
...WorkflowData
148+
}
143149
taskProxies {
144150
...TaskProxyData
145151
}
@@ -156,6 +162,14 @@ fragment PrunedDelta on Pruned {
156162
jobs
157163
}
158164

165+
# We must always request the reloaded field whenever we are requesting things
166+
# within the workflow like tasks, cycles, etc as this is used to rebuild the
167+
# store when a workflow is reloaded or restarted.
168+
fragment WorkflowData on Workflow {
169+
id
170+
reloaded
171+
}
172+
159173
# We must always request the "id" for ALL types.
160174
# The only field this view requires beyond that is the status.
161175
fragment TaskProxyData on TaskProxy {

src/views/Table.vue

+1
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ fragment PrunedDelta on Pruned {
8989

9090
fragment WorkflowData on Workflow {
9191
id
92+
reloaded
9293
}
9394

9495
fragment CyclePointData on FamilyProxy {

src/views/Tree.vue

+1
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ fragment PrunedDelta on Pruned {
103103

104104
fragment WorkflowData on Workflow {
105105
id
106+
reloaded
106107
}
107108

108109
fragment CyclePointData on FamilyProxy {

src/views/Workflows.vue

+2
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ fragment UpdatedDelta on Updated {
5656
}
5757

5858
fragment WorkflowData on Workflow {
59+
# NOTE: do not request the "reloaded" event here
60+
# (it would cause a race condition with the workflow subscription)
5961
id
6062
status
6163
statusMsg

tests/unit/services/workflow.service.spec.js

+57
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
* along with this program. If not, see <http://www.gnu.org/licenses/>.
1616
*/
1717

18+
import { createStore } from 'vuex'
19+
import storeOptions from '@/store/options'
20+
import CylcTreeCallback from '@/services/treeCallback'
1821
import sinon from 'sinon'
1922
import { print } from 'graphql/language'
2023
import gql from 'graphql-tag'
@@ -373,3 +376,57 @@ describe('WorkflowService', () => {
373376
})
374377
})
375378
})
379+
380+
describe('Global Callback', () => {
381+
it('should wipe workflow children on reloaded deltas', () => {
382+
// the callback should wipe workflow children when a "reloaded" delta is
383+
// received - see https://github.com/cylc/cylc-ui/pull/1479
384+
385+
// initiate the store
386+
const errors = {}
387+
const store = createStore(storeOptions)
388+
const callback = new CylcTreeCallback(store, errors)
389+
const cylcTree = store.state.workflows.cylcTree
390+
391+
// send an added delta which adds a workflow with one task
392+
const delta1 = {
393+
id: 123,
394+
added: {
395+
id: 123,
396+
workflow: { id: '~user/foo' },
397+
taskProxies: { id: '~user/foo//1/a' }
398+
}
399+
}
400+
callback.before(delta1, store, errors)
401+
callback.onAdded(delta1.added, store, errors)
402+
403+
// the user/workflow//cycle/task should now be in the store
404+
expect(Object.keys(cylcTree.$index)).to.deep.equal([
405+
'~user',
406+
'~user/foo',
407+
'~user/foo//1',
408+
'~user/foo//1/a',
409+
])
410+
411+
// send a reloaded delta which adds a new task
412+
const delta2 = {
413+
id: 234,
414+
added: {
415+
id: 234,
416+
workflow: { id: '~user/foo', reloaded: true },
417+
taskProxies: { id: '~user/foo//2/b' }
418+
}
419+
}
420+
callback.before(delta2, store, errors)
421+
callback.onUpdated(delta2.added, store, errors)
422+
423+
// the cycle "1" and task "1/a" should be gone from the store
424+
// without the need for an explicit "pruned" delta
425+
expect(Object.keys(cylcTree.$index)).to.deep.equal([
426+
'~user',
427+
'~user/foo',
428+
'~user/foo//2',
429+
'~user/foo//2/b',
430+
])
431+
})
432+
})

0 commit comments

Comments
 (0)