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 #5910

Closed
AndreiEres opened this issue Oct 2, 2024 · 0 comments
Closed

Rename QueueEvent::StartWork #5910

AndreiEres opened this issue Oct 2, 2024 · 0 comments
Assignees
@AndreiEres
Labels
I4-refactor Code needs refactoring. T8-polkadot This PR/Issue is related to/affects the Polkadot network.

Comments

@AndreiEres
Copy link
Contributor

AndreiEres commented Oct 2, 2024 •
edited
Loading

If I understand correctly, when we send QueueEvent::StartWork, we have already completed the execution. It might be better to call it 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;
},

@AndreiEres AndreiEres self-assigned this Oct 2, 2024
@AndreiEres AndreiEres added I4-refactor Code needs refactoring. T8-polkadot This PR/Issue is related to/affects the Polkadot network. labels Oct 2, 2024
@AndreiEres AndreiEres mentioned this issue Oct 10, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Oct 11, 2024
@AndreiEres @actions-user
Rename QueueEvent::StartWork (#6015)
e5ccc00 
# 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`.


https://github.com/paritytech/polkadot-sdk/blob/c52675efdc05e181ddcec72d3bd425dc0a89d622/polkadot/node/core/pvf/src/execute/queue.rs#L632-L646


https://github.com/paritytech/polkadot-sdk/blob/c52675efdc05e181ddcec72d3bd425dc0a89d622/polkadot/node/core/pvf/src/execute/queue.rs#L361-L363

Fixes #5910

## Integration

Shouldn't affect downstream projects.

---------

Co-authored-by: GitHub Action <action@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AndreiEres AndreiEres

Labels
I4-refactor Code needs refactoring. T8-polkadot This PR/Issue is related to/affects the Polkadot network.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@AndreiEres