Feature: Implement proving worker #22

ocdbytes · 2024-06-19T14:33:47Z

Added proving worker
Added proving worker test cases

unstark · 2024-06-19T19:56:40Z

crates/orchestrator/src/database/mongodb/mod.rs

+                                "$expr": {
+                                    "$and": [
+                                        { "$eq": ["$job_type", "ProofCreation"] },
+                                        { "$eq": ["$internal_id", "$$internal_id"] }


Maybe we should rename internal_id to block_id at last? :)
The relation between jobs (and the fact they can have the same ID) is non-obvious otherwise

ok cool will rename internal_id to block_id.

Hey @unstark

So I talked about this to apoorv and he told me that the interal_id field was just to make sure that for a particular job type there is a way to track the jobs internally so that's why a generic name internal_id was given to this field.

So @unstark my hypothesis was that the orchestrator can be designed in a way that we can also have jobs at the txn level later on. Maybe something like txn level proving or stream of txs to some XYZ place. So in that case, the internal_id will be the txn_hash. What are your thoughts on this? Does it feel like an overkill?

unstark · 2024-06-19T20:00:09Z

crates/orchestrator/src/database/mongodb/mod.rs

@@ -127,4 +129,53 @@ impl Database for MongoDb {
            .await
            .expect("Failed to fetch latest job by given job type"))
    }
+
+    async fn get_successful_snos_jobs_without_proving(&self) -> Result<Vec<JobItem>> {


Probably not an immediate issue, but we might need pagination here: imagine if there's a problem with the prover that lasts for quite some time.

yeah I will look into it and discuss and tell you. Can you explain a bit more on why we need pagination here in a db call ?

Just in case there is a really large amount of pending jobs :) Anyways, that's should not happen in practice, feel free to resolve

unstark · 2024-06-19T20:00:45Z

crates/orchestrator/src/database/mongodb/mod.rs

+            },
+        ];
+
+        let mut cursor = self.get_job_collection().aggregate(filter, None).await?;


Assuming jobs are sorted by internal_id here? (just to confirm that we do not need to do explicit sorting)

no we don't need sorting here as it will be already sorted result.

unstark · 2024-06-19T20:01:31Z

crates/orchestrator/src/jobs/proving_job/mod.rs

+pub struct ProvingJob;
+
+#[async_trait]
+impl Job for ProvingJob {


Will need to merge with the #6

sure will do it.

apoorvsadana · 2024-06-21T11:16:06Z

crates/orchestrator/src/database/mongodb/mod.rs

+                                "$expr": {
+                                    "$and": [
+                                        { "$eq": ["$job_type", "ProofCreation"] },
+                                        { "$eq": ["$internal_id", "$$internal_id"] }


So @unstark my hypothesis was that the orchestrator can be designed in a way that we can also have jobs at the txn level later on. Maybe something like txn level proving or stream of txs to some XYZ place. So in that case, the internal_id will be the txn_hash. What are your thoughts on this? Does it feel like an overkill?

apoorvsadana · 2024-06-21T11:22:42Z

crates/orchestrator/src/database/mod.rs

@@ -36,6 +36,8 @@ pub trait Database: Send + Sync {

    async fn update_metadata(&self, job: &JobItem, metadata: HashMap<String, String>) -> Result<()>;
    async fn get_latest_job_by_type_and_internal_id(&self, job_type: JobType) -> Result<Option<JobItem>>;
+
+    async fn get_successful_snos_jobs_without_proving(&self) -> Result<Vec<JobItem>>;


I think the database trait shouldn't have functions with the name snos or proving as that seems to leak the internal types. We should generalise the function over all types and take the type as an input. Function can look like

fn get_jobs_without_successor(job_a_type, job_a_status, job_b_type, job_b_status)

wdyt?

makes sense

apoorvsadana · 2024-06-21T11:29:57Z

crates/orchestrator/src/tests/workers/proving/mod.rs

+    }
+
+    // Queue function call simulations
+    queue


should we make this times 5 or 4?

in first case it is 5 in second case when one job is unsuccessful that's why we are using 4 here.

apoorvsadana · 2024-06-24T13:05:25Z

crates/orchestrator/src/database/mongodb/mod.rs

+    ///
+    /// job_b_status : Status of Job B / None
+    ///
+    /// **IMP** : For now Job B status implementation is pending so we can pass None


let's not take job as a param for now because it allows others to call this function by mistake. and we can make the IMP a TODO then

* feat : added snos worker implementation and unit tests * feat : added review #1 changes : added error handling for snos workers * feat : added review #1 changes : added error handling for snos workers * fix : lint * fix : lint errors * feat : added proving worker * feat : added proving worker * fix : refactor : uncomment temp changes * fix : ci fixes * fix : lint * fix : lint * feat : added complete implementation of proving job * fix : tests fix proving worker * fix : lint * feat : db generic fucntion * feat : refactoring --------- Co-authored-by: Arun Jangra <[email protected]>

Arun Jangra and others added 10 commits June 13, 2024 19:21

feat : added snos worker implementation and unit tests

2e9cf16

feat : added review #1 changes : added error handling for snos workers

10ba1e8

feat : added review #1 changes : added error handling for snos workers

2c80f7d

fix : lint

32e9584

Merge branch 'main' into feat/snos-worker

e56b3da

fix : lint errors

79e8efb

feat : added proving worker

2526f2c

feat : added proving worker

0416cfa

feat : merge main and lint fix

b6fccd0

fix : refactor : uncomment temp changes

4dc0f8d

unstark reviewed Jun 19, 2024

View reviewed changes

ocdbytes and others added 7 commits June 20, 2024 19:58

Merge branch 'main' into feat/proving-worker

45866b5

fix : ci fixes

5546839

fix : lint

8af2ecb

fix : lint

7501b49

feat : added complete implementation of proving job

9c9f438

fix : tests fix proving worker

bf2540a

fix : lint

c65693c

apoorvsadana reviewed Jun 21, 2024

View reviewed changes

feat : db generic fucntion

9358c18

apoorvsadana reviewed Jun 24, 2024

View reviewed changes

feat : refactoring

da5de60

ocdbytes merged commit 5c96c8e into main Jun 24, 2024
6 checks passed

ocdbytes deleted the feat/proving-worker branch June 24, 2024 14:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Implement proving worker #22

Feature: Implement proving worker #22

ocdbytes commented Jun 19, 2024

unstark Jun 19, 2024

ocdbytes Jun 20, 2024

ocdbytes Jun 20, 2024

apoorvsadana Jun 21, 2024

unstark Jun 19, 2024

ocdbytes Jun 20, 2024

unstark Jun 20, 2024

unstark Jun 19, 2024

ocdbytes Jun 20, 2024

unstark Jun 19, 2024

ocdbytes Jun 20, 2024

apoorvsadana Jun 21, 2024

apoorvsadana Jun 21, 2024

ocdbytes Jun 24, 2024

apoorvsadana Jun 21, 2024

ocdbytes Jun 24, 2024

apoorvsadana Jun 24, 2024

Feature: Implement proving worker #22

Feature: Implement proving worker #22

Conversation

ocdbytes commented Jun 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment