Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Use background tasks properly in candidate-validation #4002

Merged
merged 3 commits into from
Oct 4, 2021

Conversation

rphmeier
Copy link
Contributor

@rphmeier rphmeier commented Oct 3, 2021

Also does some refactoring of error handling and runtime API requests.

@rphmeier rphmeier added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Oct 3, 2021
@rphmeier rphmeier requested review from ordian and pepyakin October 3, 2021 21:40
Copy link
Member

@ordian ordian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable.

I wonder if we should be using spawn_blocking because of wasm decompression thing might be CPU heavy, but OTOH we don't want to spawn too many threads.

})
.and_then(|res| {
res.map_err(|e| {
tracing::debug!(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reasoning behind making those two debug!?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are unexpected but non-fatal - trace! is too low, but warn! is too high.

@rphmeier
Copy link
Contributor Author

rphmeier commented Oct 4, 2021

My reasoning behind not using spawn-blocking was that the heaviest part of it is the actual execution which happens in a separate task. I don't expect the decompression to be very heavy.

@rphmeier rphmeier merged commit 3477414 into master Oct 4, 2021
@rphmeier rphmeier deleted the rh-candidate-validation-parallel branch October 4, 2021 12:25
ordian added a commit that referenced this pull request Oct 4, 2021
* master:
  Add extrinsic ordering filtering (#3631)
  chore: ci list files that spellcheck finds (#3992)
  Use background tasks properly in candidate-validation (#4002)
  Fix unoccupied bitfields (#4004)
  Bump syn from 1.0.77 to 1.0.78 (#4006)
ordian added a commit that referenced this pull request Oct 5, 2021
* master: (138 commits)
  Allow an Offset to Lease Periods (#3980)
  Bump quote from 1.0.9 to 1.0.10 (#4013)
  Companion for #9834 (Transaction Priority) (#3901)
  chore: update `builder` image (#3884)
  Free disputed cores before processing bitfields (#4008)
  Make candidate validation timeouts configurable (#4001)
  Add extrinsic ordering filtering (#3631)
  chore: ci list files that spellcheck finds (#3992)
  Use background tasks properly in candidate-validation (#4002)
  Fix unoccupied bitfields (#4004)
  Bump syn from 1.0.77 to 1.0.78 (#4006)
  Bump jsonrpsee-ws-client from 0.3.0 to 0.3.1 (#3931)
  fix clock drift for assignments issued before the block (#3851)
  Remove unoccupied bit check (#3999)
  bump substrate (#4000)
  change genesis authority set for wococo-local, revert rococo-local (#3998)
  ignore irrelevant approvals in logs (#3859)
  avoid expect, on free availability core (#3994)
  preserve finalized block in active leaves (#3997)
  some tweaks to rococo-local (#3996)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants