Add runtime scheduling constraints #3689

kostko · 2021-02-10T11:37:45Z

TODO

Actually implement the new constraints (validator set membership, max nodes per entity).
Fix runtime descriptor generation CLI to properly support constraints.
Basic validation of constraints structure.
Tests.
Cleanup.

codecov · 2021-02-11T14:23:48Z

Codecov Report

Merging #3689 (c4d1f0b) into master (1133df4) will increase coverage by 0.18%.
The diff coverage is 86.71%.

@@            Coverage Diff             @@
##           master    #3689      +/-   ##
==========================================
+ Coverage   66.62%   66.80%   +0.18%     
==========================================
  Files         400      400              
  Lines       39660    39769     +109     
==========================================
+ Hits        26424    26569     +145     
+ Misses       9460     9418      -42     
- Partials     3776     3782       +6

Impacted Files	Coverage Δ
go/registry/api/runtime.go	`43.97% <ø> (-2.50%)`	⬇️
go/worker/storage/committee/checkpoint_sync.go	`73.49% <ø> (ø)`
go/scheduler/api/api.go	`59.03% <65.00%> (+7.52%)`	⬆️
go/worker/compute/executor/committee/node.go	`68.20% <66.66%> (-0.32%)`	⬇️
...o/consensus/tendermint/apps/scheduler/scheduler.go	`72.23% <80.64%> (+7.59%)`	⬆️
go/oasis-node/cmd/registry/runtime/runtime.go	`61.71% <95.65%> (+3.22%)`	⬆️
go/consensus/tendermint/apps/roothash/roothash.go	`73.95% <100.00%> (-0.18%)`	⬇️
go/consensus/tendermint/apps/scheduler/genesis.go	`31.29% <100.00%> (-0.05%)`	⬇️
go/registry/tests/tester.go	`91.60% <100.00%> (+0.12%)`	⬆️
go/roothash/api/commitment/pool.go	`70.00% <100.00%> (ø)`
... and 43 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1133df4...c4d1f0b. Read the comment docs.

go/consensus/tendermint/apps/scheduler/genesis.go

go/oasis-node/cmd/debug/txsource/workload/parallel.go

pro-wh · 2021-02-12T18:14:29Z

go/consensus/tendermint/apps/scheduler/scheduler.go

+				entropy,
+				stakeAcc,
+				entitiesEligibleForReward,
+				validatorEntities,


ℹ️ this is the one that's added

go/consensus/tendermint/apps/scheduler/scheduler.go

go/oasis-test-runner/scenario/e2e/registry_cli.go

go/oasis-test-runner/scenario/e2e/runtime/multiple_runtimes.go

go/oasis-test-runner/scenario/e2e/runtime/txsource.go

pro-wh · 2021-02-12T19:27:24Z

go/registry/api/runtime.go

@@ -113,10 +111,6 @@ func (e *ExecutorParameters) ValidateBasic() error {
 		return fmt.Errorf("round timeout too small")
 	}

-	if e.MinPoolSize < e.GroupSize+e.GroupBackupSize {
-		return fmt.Errorf("minimum pool size too small")


do these checks get reinstated elsewhere?

No, we could add them back to catch nonsensical configs.

pro-wh · 2021-02-12T19:30:58Z

go/consensus/tendermint/apps/scheduler/scheduler.go

-		backupSize = int(rt.Executor.GroupBackupSize)
-		minPoolSize = int(rt.Executor.MinPoolSize)
+		groupSizes[scheduler.RoleWorker] = int(rt.Executor.GroupSize)
+		groupSizes[scheduler.RoleBackupWorker] = int(rt.Executor.GroupBackupSize)


this makes me wonder about moving the GroupSize and GroupBackupSize into the SchedulingConstraints, like this change does with MinPoolSize. are there other places where we like having it separate like this?

He he, I had the exact same thought as well. It would definitely make things nicer.

pro-wh · 2021-02-18T22:02:52Z

go/consensus/tendermint/apps/roothash/roothash.go

-				Attribute(KeyExecutionDiscrepancyDetected, cbor.Marshal(tagV)).
-				Attribute(KeyRuntimeID, ValueRuntimeID(runtime.ID)),
-		)
+		// This was already handled above, so it should not happen.


control shouldn't reach here, or just nothing left to do? we don't return early in the if added above

The branch added above will overwrite err and the repeated TryFinalize cannot (currently) return the "discrepancy detected" error. So control shouldn't reach here.

pro-wh · 2021-02-18T22:29:17Z

go/roothash/api/commitment/pool_test.go

@@ -1095,6 +1120,7 @@ func setupDiscrepancy(
 	sks []signature.Signer,
 	committee *scheduler.Committee,
 	nl NodeLookup,
+	enoughBackupCommits bool,


ℹ️ due to some overlap between the worker and backup committees, some tests for worker bad behavior also result in missing backup commits

pro-wh

'worker' and 'backup' roles are no longer exclusive with each other. that results in pretty extensive changes.

go/oasis-test-runner/scenario/e2e/runtime/byzantine.go

kostko force-pushed the kostko/feature/rt-eligibility-constraints branch 4 times, most recently from 5a061e4 to 96db053 Compare February 11, 2021 13:31

kostko force-pushed the kostko/feature/rt-eligibility-constraints branch from 96db053 to c443d15 Compare February 12, 2021 12:06

kostko marked this pull request as ready for review February 12, 2021 12:06

kostko requested review from peterjgilbert, pro-wh, ptrus and Yawning as code owners February 12, 2021 12:06

kostko force-pushed the kostko/feature/rt-eligibility-constraints branch 2 times, most recently from e5da82b to 81a07c4 Compare February 12, 2021 12:38

pro-wh reviewed Feb 12, 2021

View reviewed changes

kostko force-pushed the kostko/feature/rt-eligibility-constraints branch 5 times, most recently from 21271e9 to 8abb171 Compare February 18, 2021 16:54

pro-wh reviewed Feb 18, 2021

View reviewed changes

pro-wh approved these changes Feb 18, 2021

View reviewed changes

ptrus reviewed Feb 19, 2021

View reviewed changes

go/oasis-test-runner/scenario/e2e/runtime/byzantine.go Show resolved Hide resolved

kostko force-pushed the kostko/feature/rt-eligibility-constraints branch from 8abb171 to 8e04fa2 Compare February 19, 2021 10:20

kostko added 4 commits February 19, 2021 13:43

Support multiple committee roles per node

3447673

Add runtime scheduling constraints

60fae63

e2e/txsource/workload: Bump timeout intervals

e9ab863

ci: Bump E2E timeout

c4d1f0b

kostko force-pushed the kostko/feature/rt-eligibility-constraints branch from 8e04fa2 to c4d1f0b Compare February 19, 2021 12:44

kostko mentioned this pull request Feb 19, 2021

Update documentation related to runtime parameters #3715

Closed

kostko merged commit 9998dea into master Feb 19, 2021

kostko deleted the kostko/feature/rt-eligibility-constraints branch February 19, 2021 13:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add runtime scheduling constraints #3689

Add runtime scheduling constraints #3689

kostko commented Feb 10, 2021 •

edited

Loading

codecov bot commented Feb 11, 2021 •

edited

Loading

pro-wh Feb 12, 2021

pro-wh Feb 12, 2021

kostko Feb 12, 2021

pro-wh Feb 12, 2021

kostko Feb 12, 2021

pro-wh Feb 18, 2021

kostko Feb 19, 2021 •

edited

Loading

pro-wh Feb 18, 2021

pro-wh left a comment

Add runtime scheduling constraints #3689

Add runtime scheduling constraints #3689

Conversation

kostko commented Feb 10, 2021 • edited Loading

codecov bot commented Feb 11, 2021 • edited Loading

Codecov Report

pro-wh Feb 12, 2021

Choose a reason for hiding this comment

pro-wh Feb 12, 2021

Choose a reason for hiding this comment

kostko Feb 12, 2021

Choose a reason for hiding this comment

pro-wh Feb 12, 2021

Choose a reason for hiding this comment

kostko Feb 12, 2021

Choose a reason for hiding this comment

pro-wh Feb 18, 2021

Choose a reason for hiding this comment

kostko Feb 19, 2021 • edited Loading

Choose a reason for hiding this comment

pro-wh Feb 18, 2021

Choose a reason for hiding this comment

pro-wh left a comment

Choose a reason for hiding this comment

kostko commented Feb 10, 2021 •

edited

Loading

codecov bot commented Feb 11, 2021 •

edited

Loading

kostko Feb 19, 2021 •

edited

Loading