How can we speed up our pull request workflows? (Strategies stage)

[teor2345]

We want to make pull requests merge faster, but still achieve our other goals.

What can we do to speed up our CI?

If we post each suggestion as a top-level comment, they will each get their own discussion thread.

Goals

Here are our goals for reviewing, testing, and merging code:

• We want our code to enforce the consensus rules, and interface with lightwalletd
• We want to start new work based on the latest code
• PRs, CI, and merges should be easy to work with, so we can focus on code & reviews
  • If a PR makes CI fail, we don't want to merge it to main, because then everyone's PRs fail CI
  • We want to make it easy to create small PRs, and chains of PRs
  • We want PRs to merge quickly, so that work that depends on them goes smoothly
  • We want to avoid merge conflicts

(Based on #3935.)

[teor2345]

Suggestion: Speed up Zebra's syncing speed.

Our longest workflows sync Zebra from the genesis block, and take a few hours. But we know Zebra can go faster, because it can copy its state in about 15 minutes.

If we can work out what is slow, and fix the slowest bits, we can make all these workflows faster. And we can make Zebra faster for users.

[teor2345]

Timing: We might want to wait until we've merged all the lightwalletd database changes.

We've seen some performance changes during this work. So I want to make sure we're speeding up the latest state version.

(I don't expect many state changes after this sprint.)

[teor2345]

We needed the sync to be fast for the lightwalletd integration tests in #3986.

So I opened PR #4040 to speed up the sync.

[teor2345]

Suggestion: Only re-sync from genesis when it is actually needed.

We have a lot of database change PRs open right now. Each time one of them gets rebuilt, it replaces the cached state image with its own state version.

When the state version doesn't match the running PR, all the cached state tests take a few hours, because they sync from genesis.

1. We could find an image with the correct state version, and use that.

2. We might also be able to skip the state rebuild for more PRs. (We have a list of changed files that rebuild the state, but some changes don't actually change the state format.)

[conradoplg]

This seems to me the biggest time saver, see this PR for example which was a simple change but it's been running for hours because it's using the wrong cached state.

[teor2345]

This seems to me the biggest time saver

I think it's also a test coverage bug. We should be testing code that doesn't change the database format version with the old state version, to detect unintentional changes.

So I opened a PR for it, because we're about to do a bunch of database changes:
#4020

[teor2345]

I opened a PR for it, because we're about to do a bunch of database changes:
#4020

This PR got merged into main.

[teor2345]

Suggestion: Use rebase merges to main, to avoid GitHub merge conflicts

GitHub's merging algorithm sometimes detects conflicts with squashed PRs, when git rebase does not.

If we rebased PR commits on main instead, we could avoid these conflicts.
(People can still squash commits before merging if they want to.)

[teor2345]

Suggestion: Reduce build time

If we can build Zebra faster, most jobs will finish faster.

Example:

I just discovered that we're building all the compression libraries into our rocksdb instance, but we're only using snappy.

(And maybe we should be using lz4, as recommended by the rocksdb developers.)

[teor2345]

I removed the unused dependencies in PR #4082

[gustavovalverde]

I've been analyzing some behaviors with our PR workflow, this are some annotations:

1. A single PR can take up to 6 hours merging (if a Full Sync is triggered)
2. Having multiple PRs waiting for review and not having those getting into a queue at the same time, would commonly mean a serialized merge (6 hours per PR). Even if the reviews happen on the same day, based on the priority label, we could have 4+ PRs taking ~2 days just merging.
3. cargo test --locked --release --features enable-sentry --workspace --no-run does not completely caches from cargo build --locked --release --features enable-sentry --bin zebrad and this is a 20 minute build-time-wait between commits.

Possible solutions:

1. Reduce the paths or conditions being considered to trigger a full sync
2. Reduce the amount of queues. Maybe 2 queues are enough (hotfixand changes/improvements)
3. Increase the time mergify waits before queueing PRs together
4. Increase the amount of speculative_checks and batch_size to 3 and 5 respectively for all queues in mergify
5. Set allow_checks_interruption to true in mergify, as this would allow for hotfixes to be merged faster
6. Reduce the amount of time it takes to compile the Top 10 modules/libraries in build time:

Time	Module/Library
159.9	Compiling zebra-consensus v1.0.0-beta.7 (/app/zebra-consensus)
132.5	Compiling zebra-rpc v1.0.0-beta.7 (/app/zebra-rpc)
64.3	Compiling zebra-state v1.0.0-beta.7 (/app/zebra-state)
46.7	Compiling zebra-utils v1.0.0-beta.7 (/app/zebra-utils)
40	Compiling color-eyre v0.5.11
38.2	Compiling zebra-script v1.0.0-beta.7 (/app/zebra-script)
20.6	Compiling reddsa v0.0.0 (https://github.com/str4d/redjubjub.git?rev=416a6a8ebf8bd42c114c938883016c04f338de72#416a6a8e)
15.4	Compiling zcash_script v0.1.6-alpha.0 (https://github.com/ZcashFoundation/zcash_script.git?rev=270d32d192c5880f911acf21ef100caa128e6179#270d32d1)
12.1	Compiling tinytemplate v1.2.1
10.3	Compiling zebra-test v1.0.0-beta.7 (/app/zebra-test)
9.2	Compiling zcash_proofs v0.5.0 (https://github.com/ZcashFoundation/librustzcash.git?tag=0.5.1-zebra-v1.0.0-beta.4#80d5b049)
132.5	Compiling zebra-rpc v1.0.0-beta.7 (/app/zebra-rpc)
64.3	Compiling zebra-state v1.0.0-beta.7 (/app/zebra-state)
46.7	Compiling zebra-utils v1.0.0-beta.7 (/app/zebra-utils)
40	Compiling color-eyre v0.5.11
38.2	Compiling zebra-script v1.0.0-beta.7 (/app/zebra-script)
20.6	Compiling reddsa v0.0.0 (https://github.com/str4d/redjubjub.git?rev=416a6a8ebf8bd42c114c938883016c04f338de72#416a6a8e)
15.4	Compiling zcash_script v0.1.6-alpha.0 (https://github.com/ZcashFoundation/zcash_script.git?rev=270d32d192c5880f911acf21ef100caa128e6179#270d32d1)
12.1	Compiling tinytemplate v1.2.1
10.3	Compiling zebra-test v1.0.0-beta.7 (/app/zebra-test)
9.2	Compiling zcash_proofs v0.5.0 (https://github.com/ZcashFoundation/librustzcash.git?tag=0.5.1-zebra-v1.0.0-beta.4#80d5b049)

[gustavovalverde]

Comparing with my local machine, it does. Even though it doesn't change the position of the modules in the table.

Note: We can't increase the cores in GitHub unless we use self-hosted runners.

[teor2345]

Here are my initial thoughts:

• Reduce the paths or conditions being considered to trigger a full sync

Might be possible, let's check each path and see if it is needed?

• Reduce the amount of queues. Maybe 2 queues are enough (hotfixand changes/improvements)
• Increase the time mergify waits before queueing PRs together
• Increase the amount of speculative_checks and batch_size to 3 and 5 respectively for all queues in mergify

Queue failures seem rare, so these all seem like things we could do.

• Set allow_checks_interruption to true in mergify, as this would allow for hotfixes to be merged faster

Seems like a good idea, maybe we should only put critical fixes in the hotfix queue?

• Reduce the amount of time it takes to compile the Top 10 modules/libraries in build time:

Might be possible, but I'm not sure how to do it. We're trying to avoid Rust changes outside lightwalletd.

One option is getting rid of build scripts and features.

[teor2345]

• Reduce the amount of time it takes to compile the Top 10 modules/libraries in build time:

Might be possible, but I'm not sure how to do it. We're trying to avoid Rust changes outside lightwalletd.

We can also remove unused or duplicated dependencies, like in this PR:
https://github.com/ZcashFoundation/zebra/pull/4082/files#diff-062ee5c7945fb2fd6ddfa881b76c2a0240706a5846a9ab57c485bdbd9ee48980L26-R26

[gustavovalverde]

Created the following tickets as actionable items:

• ci: check if the actual paths being used by the full sync test needs improvement #4091
• ci: improve mergify's merge queue throughput #4092

[teor2345]

Does increasing the number of cores on the machine or the RAM speed up the build?

Comparing with my local machine, it does. Even though it doesn't change the position of the modules in the table.

Note: We can't increase the cores in GitHub unless we use self-hosted runners.

Can we increase the number of cores in Google Cloud builds?