Skip to content

feat: Make TaskStrategy more granular #618

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

Draft
wants to merge 1 commit into
base: snawaz/rename-optimize
Choose a base branch
from
+61 −53
Draft

feat: Make TaskStrategy more granular #618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions magicblock-committor-service/src/persist/commit_persister.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -584,14 +584,14 @@ mod tests {
persister.set_commit_id(1, &pubkey, 100).unwrap();

persister
.set_commit_strategy(100, &pubkey, CommitStrategy::Args)
.set_commit_strategy(100, &pubkey, CommitStrategy::StateArgs)
.unwrap();

let updated = persister
.get_commit_status_by_message(1, &pubkey)
.unwrap()
.unwrap();
assert_eq!(updated.commit_strategy, CommitStrategy::Args);
assert_eq!(updated.commit_strategy, CommitStrategy::StateArgs);
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions magicblock-committor-service/src/persist/db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ mod tests {
commit_type: CommitType::DataAccount,
created_at: 1000,
commit_status: CommitStatus::Pending,
commit_strategy: CommitStrategy::Args,
commit_strategy: CommitStrategy::StateArgs,
last_retried_at: 1000,
retries_count: 0,
}
Expand Down Expand Up @@ -900,7 +900,7 @@ mod tests {
let row = create_test_row(1, 100);
db.insert_commit_status_rows(&[row.clone()]).unwrap();

let new_strategy = CommitStrategy::FromBuffer;
let new_strategy = CommitStrategy::StateBuffer;
db.set_commit_strategy(100, &row.pubkey, new_strategy)
.unwrap();

Expand Down
34 changes: 18 additions & 16 deletions magicblock-committor-service/src/persist/types/commit_strategy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,39 +4,39 @@ use crate::persist::error::CommitPersistError;
pub enum CommitStrategy {
/// Args without the use of a lookup table
#[default]
Args,
StateArgs,
/// Args with the use of a lookup table
ArgsWithLookupTable,
StateArgsWithLookupTable,
/// Buffer and chunks which has the most overhead
FromBuffer,
StateBuffer,
/// Buffer and chunks with the use of a lookup table
FromBufferWithLookupTable,
StateBufferWithLookupTable,
}

impl CommitStrategy {
pub fn args(use_lookup: bool) -> Self {
if use_lookup {
Self::ArgsWithLookupTable
Self::StateArgsWithLookupTable
} else {
Self::Args
Self::StateArgs
}
}

pub fn as_str(&self) -> &str {
use CommitStrategy::*;
match self {
Args => "Args",
ArgsWithLookupTable => "ArgsWithLookupTable",
FromBuffer => "FromBuffer",
FromBufferWithLookupTable => "FromBufferWithLookupTable",
StateArgs => "StateArgs",
StateArgsWithLookupTable => "StageArgsWithLookupTable",
StateBuffer => "StageBuffer",
StateBufferWithLookupTable => "StageBufferWithLookupTable",
}
}

pub fn uses_lookup(&self) -> bool {
matches!(
self,
CommitStrategy::ArgsWithLookupTable
| CommitStrategy::FromBufferWithLookupTable
CommitStrategy::StateArgsWithLookupTable
| CommitStrategy::StateBufferWithLookupTable
)
}
}
Expand All @@ -45,10 +45,12 @@ impl TryFrom<&str> for CommitStrategy {
type Error = CommitPersistError;
fn try_from(value: &str) -> Result<Self, CommitPersistError> {
match value {
"Args" => Ok(Self::Args),
"ArgsWithLookupTable" => Ok(Self::ArgsWithLookupTable),
"FromBuffer" => Ok(Self::FromBuffer),
"FromBufferWithLookupTable" => Ok(Self::FromBufferWithLookupTable),
"StateArgs" => Ok(Self::StateArgs),
"StateArgsWithLookupTable" => Ok(Self::StateArgsWithLookupTable),
"StageBuffer" => Ok(Self::StateBuffer),
"StageBufferWithLookupTable" => {
Ok(Self::StateBufferWithLookupTable)
}
_ => Err(CommitPersistError::InvalidCommitStrategy(
value.to_string(),
)),
Expand Down
8 changes: 4 additions & 4 deletions magicblock-committor-service/src/tasks/task_visitors/persistor_visitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ where
};

let commit_strategy = if uses_lookup_tables {
CommitStrategy::ArgsWithLookupTable
CommitStrategy::StateArgsWithLookupTable
} else {
CommitStrategy::Args
CommitStrategy::StateArgs
};

if let Err(err) = self.persistor.set_commit_strategy(
Expand All @@ -57,9 +57,9 @@ where
PersistorContext::PersistStrategy { uses_lookup_tables } => {
let BufferTaskType::Commit(ref commit_task) = task.task_type;
let commit_strategy = if uses_lookup_tables {
CommitStrategy::FromBufferWithLookupTable
CommitStrategy::StateBufferWithLookupTable
} else {
CommitStrategy::FromBuffer
CommitStrategy::StateBuffer
};

if let Err(err) = self.persistor.set_commit_strategy(
Expand Down
64 changes: 35 additions & 29 deletions test-integration/test-committor-service/tests/test_ix_commit_local.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,42 +66,42 @@ fn expect_strategies(
// # see the PR #575 for more context.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_ix_commit_single_account_100_bytes() {
commit_single_account(100, CommitStrategy::Args, false).await;
commit_single_account(100, CommitStrategy::StateArgs, false).await;
}

// TODO (snawaz): use #[tokio::test] once CommitTask::new() stops using blocking RpcClient
// # see the PR #575 for more context.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_ix_commit_single_account_100_bytes_and_undelegate() {
commit_single_account(100, CommitStrategy::Args, true).await;
commit_single_account(100, CommitStrategy::StateArgs, true).await;
}

// TODO (snawaz): use #[tokio::test] once CommitTask::new() stops using blocking RpcClient
// # see the PR #575 for more context.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_ix_commit_single_account_800_bytes() {
commit_single_account(800, CommitStrategy::FromBuffer, false).await;
commit_single_account(800, CommitStrategy::StateBuffer, false).await;
}

// TODO (snawaz): use #[tokio::test] once CommitTask::new() stops using blocking RpcClient
// # see the PR #575 for more context.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_ix_commit_single_account_800_bytes_and_undelegate() {
commit_single_account(800, CommitStrategy::FromBuffer, true).await;
commit_single_account(800, CommitStrategy::StateBuffer, true).await;
}

// TODO (snawaz): use #[tokio::test] once CommitTask::new() stops using blocking RpcClient
// # see the PR #575 for more context.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_ix_commit_single_account_one_kb() {
commit_single_account(1024, CommitStrategy::FromBuffer, false).await;
commit_single_account(1024, CommitStrategy::StateBuffer, false).await;
}

// TODO (snawaz): use #[tokio::test] once CommitTask::new() stops using blocking RpcClient
// # see the PR #575 for more context.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_ix_commit_single_account_ten_kb() {
commit_single_account(10 * 1024, CommitStrategy::FromBuffer, false).await;
commit_single_account(10 * 1024, CommitStrategy::StateBuffer, false).await;
}

async fn commit_single_account(
Expand Down Expand Up @@ -176,7 +176,7 @@ async fn test_ix_commit_two_accounts_1kb_2kb() {
&[1024, 2048],
1,
false,
expect_strategies(&[(CommitStrategy::Args, 2)]),
expect_strategies(&[(CommitStrategy::StateArgs, 2)]),
)
.await;
}
Expand All @@ -190,7 +190,7 @@ async fn test_ix_commit_two_accounts_512kb() {
&[512, 512],
1,
false,
expect_strategies(&[(CommitStrategy::Args, 2)]),
expect_strategies(&[(CommitStrategy::StateArgs, 2)]),
)
.await;
}
Expand All @@ -204,7 +204,7 @@ async fn test_ix_commit_three_accounts_512kb() {
&[512, 512, 512],
1,
false,
expect_strategies(&[(CommitStrategy::Args, 3)]),
expect_strategies(&[(CommitStrategy::StateArgs, 3)]),
)
.await;
}
Expand All @@ -218,7 +218,7 @@ async fn test_ix_commit_six_accounts_512kb() {
&[512, 512, 512, 512, 512, 512],
1,
false,
expect_strategies(&[(CommitStrategy::Args, 6)]),
expect_strategies(&[(CommitStrategy::StateArgs, 6)]),
)
.await;
}
Expand All @@ -232,7 +232,7 @@ async fn test_ix_commit_four_accounts_1kb_2kb_5kb_10kb_single_bundle() {
&[1024, 2 * 1024, 5 * 1024, 10 * 1024],
1,
false,
expect_strategies(&[(CommitStrategy::Args, 4)]),
expect_strategies(&[(CommitStrategy::StateArgs, 4)]),
)
.await;
}
Expand All @@ -241,8 +241,11 @@ async fn test_ix_commit_four_accounts_1kb_2kb_5kb_10kb_single_bundle() {
// # see the PR #575 for more context.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_commit_20_accounts_1kb_bundle_size_2() {
commit_20_accounts_1kb(2, expect_strategies(&[(CommitStrategy::Args, 20)]))
.await;
commit_20_accounts_1kb(
2,
expect_strategies(&[(CommitStrategy::StateArgs, 20)]),
)
.await;
}

// TODO (snawaz): use #[tokio::test] once CommitTask::new() stops using blocking RpcClient
Expand All @@ -251,7 +254,7 @@ async fn test_commit_20_accounts_1kb_bundle_size_2() {
async fn test_commit_5_accounts_1kb_bundle_size_3() {
commit_5_accounts_1kb(
3,
expect_strategies(&[(CommitStrategy::Args, 5)]),
expect_strategies(&[(CommitStrategy::StateArgs, 5)]),
false,
)
.await;
Expand All @@ -265,8 +268,8 @@ async fn test_commit_5_accounts_1kb_bundle_size_3_undelegate_all() {
3,
expect_strategies(&[
// Intent fits in 1 TX only with ALT, see IntentExecutorImpl::try_unite_tasks
(CommitStrategy::FromBufferWithLookupTable, 3),
(CommitStrategy::Args, 2),
(CommitStrategy::StateBufferWithLookupTable, 3),
(CommitStrategy::StateArgs, 2),
]),
true,
)
Expand All @@ -280,8 +283,8 @@ async fn test_commit_5_accounts_1kb_bundle_size_4() {
commit_5_accounts_1kb(
4,
expect_strategies(&[
(CommitStrategy::Args, 1),
(CommitStrategy::FromBufferWithLookupTable, 4),
(CommitStrategy::StateArgs, 1),
(CommitStrategy::StateBufferWithLookupTable, 4),
]),
false,
)
Expand All @@ -295,8 +298,8 @@ async fn test_commit_5_accounts_1kb_bundle_size_4_undelegate_all() {
commit_5_accounts_1kb(
4,
expect_strategies(&[
(CommitStrategy::Args, 1),
(CommitStrategy::FromBufferWithLookupTable, 4),
(CommitStrategy::StateArgs, 1),
(CommitStrategy::StateBufferWithLookupTable, 4),
]),
true,
)
Expand All @@ -309,7 +312,7 @@ async fn test_commit_5_accounts_1kb_bundle_size_4_undelegate_all() {
async fn test_commit_5_accounts_1kb_bundle_size_5_undelegate_all() {
commit_5_accounts_1kb(
5,
expect_strategies(&[(CommitStrategy::FromBufferWithLookupTable, 5)]),
expect_strategies(&[(CommitStrategy::StateBufferWithLookupTable, 5)]),
true,
)
.await;
Expand All @@ -319,8 +322,11 @@ async fn test_commit_5_accounts_1kb_bundle_size_5_undelegate_all() {
// # see the PR #575 for more context.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_commit_20_accounts_1kb_bundle_size_3() {
commit_20_accounts_1kb(3, expect_strategies(&[(CommitStrategy::Args, 20)]))
.await;
commit_20_accounts_1kb(
3,
expect_strategies(&[(CommitStrategy::StateArgs, 20)]),
)
.await;
}

// TODO (snawaz): use #[tokio::test] once CommitTask::new() stops using blocking RpcClient
Expand All @@ -329,7 +335,7 @@ async fn test_commit_20_accounts_1kb_bundle_size_3() {
async fn test_commit_20_accounts_1kb_bundle_size_4() {
commit_20_accounts_1kb(
4,
expect_strategies(&[(CommitStrategy::FromBufferWithLookupTable, 20)]),
expect_strategies(&[(CommitStrategy::StateBufferWithLookupTable, 20)]),
)
.await;
}
Expand All @@ -341,9 +347,9 @@ async fn test_commit_20_accounts_1kb_bundle_size_6() {
commit_20_accounts_1kb(
6,
expect_strategies(&[
(CommitStrategy::FromBufferWithLookupTable, 18),
(CommitStrategy::StateBufferWithLookupTable, 18),
// Two accounts don't make it into the bundles of size 6
(CommitStrategy::Args, 2),
(CommitStrategy::StateArgs, 2),
]),
)
.await;
Expand All @@ -355,7 +361,7 @@ async fn test_commit_20_accounts_1kb_bundle_size_6() {
async fn test_commit_20_accounts_1kb_bundle_size_20() {
commit_20_accounts_1kb(
20,
expect_strategies(&[(CommitStrategy::FromBufferWithLookupTable, 20)]),
expect_strategies(&[(CommitStrategy::StateBufferWithLookupTable, 20)]),
)
.await;
}
Expand All @@ -369,7 +375,7 @@ async fn test_commit_8_accounts_1kb_bundle_size_8() {
expect_strategies(&[
// Four accounts don't make it into the bundles of size 8, but
// that bundle also needs lookup tables
(CommitStrategy::FromBufferWithLookupTable, 8),
(CommitStrategy::StateBufferWithLookupTable, 8),
]),
)
.await;
Expand All @@ -384,7 +390,7 @@ async fn test_commit_20_accounts_1kb_bundle_size_8() {
expect_strategies(&[
// Four accounts don't make it into the bundles of size 8, but
// that bundle also needs lookup tables
(CommitStrategy::FromBufferWithLookupTable, 20),
(CommitStrategy::StateBufferWithLookupTable, 20),
]),
)
.await;
Expand Down