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

Add a request pool #1137

Merged
merged 20 commits into from
Sep 7, 2023
Merged

Add a request pool #1137

merged 20 commits into from
Sep 7, 2023

Conversation

DaughterOfMars
Copy link

Description of change

This PR adds a request pool to the client, which simply waits for a limited resource to arrive on a channel before calling any client request. This is implemented using an extension trait for Future which first waits for permission from the pool to make the request, then calls the original future, then returns the permission to the pool.

Links to any relevant issues

Closes #1100

Type of change

Choose a type of change, and delete any options that are not relevant.

  • Bug fix (a non-breaking change which fixes an issue)
  • Enhancement (a non-breaking change which adds functionality)

@DaughterOfMars DaughterOfMars changed the title Feat/request pool Add a request pool Sep 5, 2023
Copy link
Member

@thibault-martinez thibault-martinez left a comment

Choose a reason for hiding this comment

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

Changelog worthy

Copy link
Member

@Thoralf-M Thoralf-M left a comment

Choose a reason for hiding this comment

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

Just blocks forever when amount of parallel requests is > max_parallel_api_requests

Build with .with_max_parallel_api_requests(2) and then sync an account

2023-09-06 09:12:34 (UTC) iota_sdk::wallet::account::operations::syncing::addresses::output_ids::alias_foundry DEBUG [SYNC] get_alias_and_foundry_output_ids
2023-09-06 09:12:34 (UTC) reqwest::connect                           DEBUG starting new connection: https://api.testnet.shimmer.network/
2023-09-06 09:12:34 (UTC) reqwest::connect                           DEBUG starting new connection: https://api.testnet.shimmer.network/
2023-09-06 09:12:34 (UTC) iota_sdk::client::node_manager::http_client DEBUG GET: 186 ms for 200 OK https://api.testnet.shimmer.network/api/indexer/v1/outputs/basic?expirationReturnAddress=rms1qqhfjcm5gg77l9m6lmqsgr42c5drwtt9dexhpuky0kd8fhccvg8uj9k9l7h
2023-09-06 09:12:34 (UTC) iota_sdk::client::node_manager::http_client DEBUG GET: 188 ms for 200 OK https://api.testnet.shimmer.network/api/indexer/v1/outputs/basic?address=rms1qqhfjcm5gg77l9m6lmqsgr42c5drwtt9dexhpuky0kd8fhccvg8uj9k9l7h

and then nothing happens anymore (after some time the node syncing request, but syncing never continues)

Full example to reproduce

use iota_sdk::{
    client::{
        constants::SHIMMER_COIN_TYPE,
        secret::{mnemonic::MnemonicSecretManager, SecretManager},
    },
    wallet::{ClientOptions, Result, Wallet},
};

#[tokio::main]
async fn main() -> Result<()> {
    dotenvy::dotenv().ok();

    let logger_output_config = fern_logger::LoggerOutputConfigBuilder::new()
        .name("example.log")
        .target_exclusions(&["h2", "hyper", "rustls"])
        .level_filter(log::LevelFilter::Debug);
    let config = fern_logger::LoggerConfig::build()
        .with_output(logger_output_config)
        .finish();
    fern_logger::logger_init(config).unwrap();

    let client_options = ClientOptions::new()
        .with_node(&std::env::var("NODE_URL").unwrap())?
        .with_max_parallel_api_requests(2);
    let secret_manager = MnemonicSecretManager::try_from_mnemonic(iota_sdk::client::generate_mnemonic()?)?;
    let wallet = Wallet::builder()
        .with_secret_manager(SecretManager::Mnemonic(secret_manager))
        .with_client_options(client_options)
        .with_coin_type(SHIMMER_COIN_TYPE)
        .finish()
        .await?;

    let account = wallet.create_account().with_alias("Alice").finish().await?;

    println!("Syncing account");
    account.sync(None).await?;
    Ok(())
}

Copy link
Member

@Thoralf-M Thoralf-M left a comment

Choose a reason for hiding this comment

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

Works now, but please also add maxParallelApiRequests to the bindings ClientOptions

Thoralf-M
Thoralf-M previously approved these changes Sep 6, 2023
Thoralf-M
Thoralf-M previously approved these changes Sep 6, 2023
sdk/CHANGELOG.md Outdated Show resolved Hide resolved
Thoralf-M
Thoralf-M previously approved these changes Sep 6, 2023
@thibault-martinez thibault-martinez merged commit 7d89459 into develop Sep 7, 2023
23 of 24 checks passed
@thibault-martinez thibault-martinez deleted the feat/request-pool branch September 7, 2023 09:23
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.

Provide option to limit requests per second
4 participants