Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rename QueueEvent::StartWork #6015

Merged
merged 5 commits into from
Oct 11, 2024
Merged

Rename QueueEvent::StartWork #6015

merged 5 commits into from
Oct 11, 2024

Conversation

AndreiEres
Copy link
Contributor

@AndreiEres AndreiEres commented Oct 10, 2024 •
edited
Loading

Description

When we send QueueEvent::StartWork, we have already completed the execution. This may be a leftover of a previous logic change. Currently, the name is misleading, so it would be better to rename it to FinishWork.

polkadot-sdk/polkadot/node/core/pvf/src/execute/queue.rs

Lines 632 to 646 in c52675e

queue.mux.push(
async move {
let _timer = execution_timer;
let result = super::worker_interface::start_work(
idle,
job.artifact.clone(),
job.exec_timeout,
job.pvd,
job.pov,
)
.await;
QueueEvent::StartWork(worker, result, job.artifact.id, job.result_tx)
}
.boxed(),
);

polkadot-sdk/polkadot/node/core/pvf/src/execute/queue.rs

Lines 361 to 363 in c52675e

QueueEvent::StartWork(worker, outcome, artifact_id, result_tx) => {
handle_job_finish(queue, worker, outcome, artifact_id, result_tx).await;
},

Fixes #5910

Integration

Shouldn't affect downstream projects.

@AndreiEres AndreiEres added T0-node This PR/Issue is related to the topic “node”. I4-refactor Code needs refactoring. T8-polkadot This PR/Issue is related to/affects the Polkadot network. labels Oct 10, 2024
@AndreiEres
Copy link
Contributor Author

/cmd prdoc --audience node_dev --bump patch

@AndreiEres AndreiEres changed the base branch from AndreiEres/issue5530 to master October 10, 2024 15:10
s0me0ne-unkn0wn
s0me0ne-unkn0wn approved these changes Oct 11, 2024
View reviewed changes
Copy link
Contributor

@s0me0ne-unkn0wn s0me0ne-unkn0wn left a comment

Choose a reason for hiding this comment

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

👍

sandreim
sandreim approved these changes Oct 11, 2024
View reviewed changes
Copy link
Contributor

@sandreim sandreim left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM! 👍🏼

@AndreiEres AndreiEres added this pull request to the merge queue Oct 11, 2024
Merged via the queue into master with commit e5ccc00 Oct 11, 2024
201 checks passed
@AndreiEres AndreiEres deleted the AndreiEres/issue5910-Rename-QueueEventStartWork branch October 11, 2024 12:28
ordian added a commit that referenced this pull request Oct 11, 2024
@ordian
Merge branch 'master' into ao-fix-issue-64
2b29d09 
* master: (28 commits)
  `substrate-node`: removed excessive polkadot-sdk features (#5925)
  Rename QueueEvent::StartWork (#6015)
  [ci] Remove quick-benchmarks-omni from GitLab (#6014)
  Set larger timeout for cmd.yml (#6006)
  Fix `0003-beefy-and-mmr` test (#6003)
  Remove redundant XCMs from dry run's forwarded xcms (#5913)
  Add RadiumBlock bootnodes to Coretime Polkadot Chain spec (#5967)
  Bump strum from 0.26.2 to 0.26.3 (#5943)
  Add PVF execution priority (#4837)
  Snowbridge V2 docs (#5902)
  Fix u256 conversion in BABE (#5994)
  [ci] Move test-linux-stable-no-try-runtime to GHA (#5979)
  Bump PoV request timeout (#5924)
  [Release/CI] Github flow to build `polkadot`/`polkadot-parachain` rc binaries and deb package (#5963)
  [ci] Remove short-benchmarks from Gitlab (#5988)
  Disable flaky tests reported in 5972/5973/5974 (#5976)
  Bump some dependencies (#5886)
  bump zombienet version and set request for k8s (#5968)
  [omni-bencher] Make all runtimes work (#5872)
  Omni-Node renamings (#5915)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@s0me0ne-unkn0wn s0me0ne-unkn0wn s0me0ne-unkn0wn approved these changes

@sandreim sandreim sandreim approved these changes

Assignees
No one assigned
Labels
I4-refactor Code needs refactoring. T0-node This PR/Issue is related to the topic “node”. T8-polkadot This PR/Issue is related to/affects the Polkadot network.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AndreiEres @s0me0ne-unkn0wn @sandreim @actions-user