[executor] Simplify Logic + Speed Up Tests #831
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
patrick-ogrady
requested review from
samliok,
dboehm-avalabs and
richardpringle
as code owners
patrick-ogrady
commented
Apr 12, 2024
| // baseDependencies must be greater than the maximum number of dependencies | ||
| // any single task could have. This is used to ensure a dependent task | ||
| // does not begin executing a task until all dependencies have been enqueued. | ||
| const baseDependencies = 100_000_000 |
There was a problem hiding this comment.
@wlawt I think this was the main nuance you didn't include in your branch
patrick-ogrady
commented
Apr 12, 2024
| @@ -28,23 +33,17 @@ type Executor struct { | |||
| outstanding sync.WaitGroup | |||
| executable chan *task | |||
|
|
|||
| tasks map[int]*task | |||
There was a problem hiding this comment.
When possible, we should avoid ever keeping a central map we use for lookup (as it requires synchronization).
|
Load tests are stuck on waiting for |
|
217 (in this run) is never executed: |
|
load test is failing with: |
wlawt
approved these changes
Apr 12, 2024
patrick-ogrady
added a commit
that referenced
this pull request
Apr 17, 2024
…ys (#814) * [executor] refactor worker func * [executor] renaming * [executor] use blocking/dependency type from programmatic-deploy fork * [executor] add key type support * wip * wip * switch statements * programmatic deploy fork + concurrent reads work * sequential writes work * w->r->r works * r->r->w works * w->r->r...w->r->r works * r->r->w...r->r->w works * all unit tests pass * cleanup * coverage 97% * cleanup switch * switch comments * self review * go mod tidy * add done prints to tests * rename isAllocateWrite * remove id from task * use dummy dep to keep track of all keys * simplify for loop in adding bt dep * fix integration bug and add unit test for it * fix race condition with write-after-read(s) potentially * run unit tests over multiple iterations * cut down on unit testing time and add comments * simplify num times we call add blocking if not exec * new way to keep track of concurrent Reads * self review * have unit tests run under max time * [executor] Simplify Logic + Speed Up Tests (#831) * first pass * need to set higher base dependencies * remove extra logging from tests * use right var * tests passing * cleanup task loop * make tests faster * ensure arr order matches * add more comments * tests are passing * ensure we actually stop building early * add comment about failure case * add comment about deadlock * better var names * add larger unit tests * ignore lint for rand * add unique and conflicting keys randomly to txs * fix for loops * use max function * make conflict keys 100 and pick 1-5 * make num slow chan consistent * use set.Contains to speed up tests * random perm for each unique key * group var names * use numTxs in generating blocking txs * increase num conflict keys for concurrent Reads and Writes test * [executor] multi-key conflict bug (#837) * random perm per conflict key * fix dont block on ourself and edit TestTwoConflictKeys to track this * keep reading/readers relationship * make reading a map * comments * clarify use of map for reading * simplify rng for conflict and unique perm * random perm per conflict key * make maxDep a param * add maxDep as const in chain * placement of maxDep comment --------- Co-authored-by: Patrick O'Grady <prohb125@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.