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.

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

feat(Prover CLI): requeue cmd #1719

Open
wants to merge 40 commits into
base: main
Choose a base branch
from
Open

Conversation

ilitteri
Copy link
Member

@ilitteri ilitteri commented Apr 18, 2024

What ❔

Adds a requeue command that requeues a given batch if it's stuck. It looks at all jobs that are stuck and requeues them. A job is stuck if it has a large number of attempts and is not successful.

For now, the attempts are set via a cmd flag (--max-attempts) which if not set, is 10 by default.

Usage Example

cd prover/prover_cli
zk f cargo run --release requeue --batch 1

Why ❔

We want to be able to requeue a stuck batch with the CLI.

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Code has been formatted via zk fmt and zk lint.
  • Spellcheck has been run via zk spellcheck.
  • Linkcheck has been run via zk linkcheck.

@ilitteri
Copy link
Member Author

ilitteri commented Apr 18, 2024

@EmilLuta we can discuss this during the daily, but I think I'm missing a --batch flag to avoid requeuing multiple batches jobs. Also, I'm not sure if this is the same as the restart cmd

Copy link
Contributor

@EmilLuta EmilLuta left a comment

Choose a reason for hiding this comment

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

So far, so good. As we chatted during daily, let's incorporate changes for the requeuer.

@ilitteri ilitteri changed the title feat(Prover CLI): requeue cmd scaffolding + requeue basic-witness-generator-jobs cmd feat(Prover CLI): requeue cmd Apr 25, 2024
@ilitteri ilitteri requested a review from EmilLuta April 25, 2024 19:17
@ilitteri
Copy link
Member Author

I think the spellcheck error is not related to this PR:

error: spellcheck(Hunspell)
   --> /Users/ivanlitteri/Repositories/matter-labs/zksync-era/core/node/node_framework/src/service/runnables.rs:31
    |
 31 | ..rns a reference to created vec to satisfy the `.field` me..
    |                              ^^^
    | - Vec, vex, vecs, sec, vac, rec, vet, or one of 4 others
    |
    |   Possible spelling mistake found.

Error occurred during spell checking: Child process exited with code 1

Copy link
Contributor

@EmilLuta EmilLuta left a comment

Choose a reason for hiding this comment

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

LGTM. Left a bunch of nits.

prover/prover_cli/src/cli.rs Show resolved Hide resolved
prover/prover_dal/src/fri_witness_generator_dal.rs Outdated Show resolved Hide resolved
prover/prover_dal/src/fri_proof_compressor_dal.rs Outdated Show resolved Hide resolved
prover/prover_dal/src/fri_prover_dal.rs Outdated Show resolved Hide resolved
prover/prover_dal/src/fri_witness_generator_dal.rs Outdated Show resolved Hide resolved
prover/prover_cli/src/commands/requeue.rs Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants