Skip to content

Commit

Permalink
[mvr] Cherry pick mvr external resolution (#20633) (#20648)
Browse files Browse the repository at this point in the history
## Description 

To resolve the mvr table on testnet, we consult the mainnet graphql
service. We previously observed some slowness around dns lookup, so this
metric will help track whether the issue persists. Also added to
`sui-graphql-rpc` crate.

## Test plan 

How did you test the new or updated feature?

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
- [ ] REST API:

## Description 

Describe the changes or additions included in this PR.

## Test plan 

How did you test the new or updated feature?

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
- [ ] REST API:
  • Loading branch information
wlmyng authored Dec 16, 2024
1 parent 84f68bd commit 851b6a6
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 26 deletions.
36 changes: 24 additions & 12 deletions crates/sui-graphql-rpc/src/data/move_registry_data_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::{collections::HashMap, str::FromStr, sync::Arc};
use async_graphql::dataloader::{DataLoader, Loader};
use serde::{Deserialize, Serialize};

use crate::metrics::Metrics;
use crate::{
config::MoveRegistryConfig,
error::Error,
Expand All @@ -26,17 +27,19 @@ const QUERY_FRAGMENT: &str =
pub(crate) struct ExternalNamesLoader {
client: reqwest::Client,
config: MoveRegistryConfig,
metrics: Metrics,
}

/// Helper types for accessing a shared `DataLoader` instance.
#[derive(Clone)]
pub(crate) struct MoveRegistryDataLoader(pub Arc<DataLoader<ExternalNamesLoader>>);

impl ExternalNamesLoader {
pub(crate) fn new(config: MoveRegistryConfig) -> Self {
pub(crate) fn new(config: MoveRegistryConfig, metrics: Metrics) -> Self {
Self {
client: reqwest::Client::new(),
config,
metrics,
}
}

Expand Down Expand Up @@ -71,9 +74,9 @@ impl ExternalNamesLoader {
}

impl MoveRegistryDataLoader {
pub(crate) fn new(config: MoveRegistryConfig) -> Self {
pub(crate) fn new(config: MoveRegistryConfig, metrics: Metrics) -> Self {
let batch_size = config.page_limit as usize;
let data_loader = DataLoader::new(ExternalNamesLoader::new(config), tokio::spawn)
let data_loader = DataLoader::new(ExternalNamesLoader::new(config, metrics), tokio::spawn)
.max_batch_size(batch_size);
Self(Arc::new(data_loader))
}
Expand Down Expand Up @@ -102,15 +105,24 @@ impl Loader<Name> for ExternalNamesLoader {
variables: serde_json::Value::Null,
};

let res = self
.client
.post(api_url)
.json(&request_body)
.send()
.await
.map_err(|e| {
Error::MoveNameRegistry(MoveRegistryError::FailedToQueryExternalApi(e.to_string()))
})?;
let res = {
let _timer_guard = self
.metrics
.app_metrics
.external_mvr_resolution_latency
.start_timer();

self.client
.post(api_url)
.json(&request_body)
.send()
.await
.map_err(|e| {
Error::MoveNameRegistry(MoveRegistryError::FailedToQueryExternalApi(
e.to_string(),
))
})?
};

if !res.status().is_success() {
return Err(Error::MoveNameRegistry(
Expand Down
24 changes: 24 additions & 0 deletions crates/sui-graphql-rpc/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const DB_QUERY_COST_BUCKETS: &[f64] = &[
pub(crate) struct Metrics {
pub db_metrics: Arc<DBMetrics>,
pub request_metrics: Arc<RequestMetrics>,
pub app_metrics: Arc<AppMetrics>,
}

#[derive(Clone)]
Expand Down Expand Up @@ -81,14 +82,23 @@ pub(crate) struct RequestMetrics {
pub inflight_requests: Gauge,
}

#[derive(Clone)]
pub(crate) struct AppMetrics {
/// The time it takes for the non-mainnet graphql service to resolve the mvr object from
/// mainnet.
pub external_mvr_resolution_latency: Histogram,
}

impl Metrics {
pub(crate) fn new(registry: &Registry) -> Self {
let db_metrics = DBMetrics::new(registry);
let request_metrics = RequestMetrics::new(registry);
let app_metrics = AppMetrics::new(registry);

Self {
db_metrics: Arc::new(db_metrics),
request_metrics: Arc::new(request_metrics),
app_metrics: Arc::new(app_metrics),
}
}

Expand Down Expand Up @@ -253,6 +263,20 @@ impl RequestMetrics {
}
}

impl AppMetrics {
pub(crate) fn new(registry: &Registry) -> Self {
Self {
external_mvr_resolution_latency: register_histogram_with_registry!(
"external_mvr_resolution_latency",
"The time it takes for the non-mainnet graphql service to resolve the mvr object from mainnet",
LATENCY_SEC_BUCKETS.to_vec(),
registry,
)
.unwrap(),
}
}
}

/// When an error occurs, GraphQL returns a vector of PathSegments,
/// that we can use to retrieve the last node which contains the error.
pub(crate) fn query_label_for_error(query: &[PathSegment]) -> String {
Expand Down
5 changes: 4 additions & 1 deletion crates/sui-graphql-rpc/src/server/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,10 @@ impl ServerBuilder {
.context_data(metrics.clone())
.context_data(config.clone())
.context_data(move_registry_config.clone())
.context_data(MoveRegistryDataLoader::new(move_registry_config));
.context_data(MoveRegistryDataLoader::new(
move_registry_config,
metrics.clone(),
));

if config.internal_features.feature_gate {
builder = builder.extension(FeatureGate);
Expand Down
10 changes: 10 additions & 0 deletions crates/sui-mvr-graphql-rpc/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# sui-mvr-graphql-rpc

## Maintaining parity with sui-graphql-rpc crate
Generates a series of patch files from the named commit to the current HEAD, and rewrites the paths to the destination crate.
```
# Create patch with stripped paths
git format-patch <commit-hash>..HEAD --stdout -- ./crates/sui-mvr-graphql-rpc | sed 's|crates/sui-mvr-graphql-rpc/|crates/sui-graphql-rpc/|g' > patches.diff
# Apply the patch
git apply patches.diff
```

## Dev setup
Note that we use compilation flags to determine the backend for Diesel. If you're using VS Code, make sure to update settings.json with the appropriate features - there should at least be a "pg_backend" (or other backend.)
```
Expand Down
36 changes: 24 additions & 12 deletions crates/sui-mvr-graphql-rpc/src/data/move_registry_data_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::{collections::HashMap, str::FromStr, sync::Arc};
use async_graphql::dataloader::{DataLoader, Loader};
use serde::{Deserialize, Serialize};

use crate::metrics::Metrics;
use crate::{
config::MoveRegistryConfig,
error::Error,
Expand All @@ -26,17 +27,19 @@ const QUERY_FRAGMENT: &str =
pub(crate) struct ExternalNamesLoader {
client: reqwest::Client,
config: MoveRegistryConfig,
metrics: Metrics,
}

/// Helper types for accessing a shared `DataLoader` instance.
#[derive(Clone)]
pub(crate) struct MoveRegistryDataLoader(pub Arc<DataLoader<ExternalNamesLoader>>);

impl ExternalNamesLoader {
pub(crate) fn new(config: MoveRegistryConfig) -> Self {
pub(crate) fn new(config: MoveRegistryConfig, metrics: Metrics) -> Self {
Self {
client: reqwest::Client::new(),
config,
metrics,
}
}

Expand Down Expand Up @@ -71,9 +74,9 @@ impl ExternalNamesLoader {
}

impl MoveRegistryDataLoader {
pub(crate) fn new(config: MoveRegistryConfig) -> Self {
pub(crate) fn new(config: MoveRegistryConfig, metrics: Metrics) -> Self {
let batch_size = config.page_limit as usize;
let data_loader = DataLoader::new(ExternalNamesLoader::new(config), tokio::spawn)
let data_loader = DataLoader::new(ExternalNamesLoader::new(config, metrics), tokio::spawn)
.max_batch_size(batch_size);
Self(Arc::new(data_loader))
}
Expand Down Expand Up @@ -102,15 +105,24 @@ impl Loader<Name> for ExternalNamesLoader {
variables: serde_json::Value::Null,
};

let res = self
.client
.post(api_url)
.json(&request_body)
.send()
.await
.map_err(|e| {
Error::MoveNameRegistry(MoveRegistryError::FailedToQueryExternalApi(e.to_string()))
})?;
let res = {
let _timer_guard = self
.metrics
.app_metrics
.external_mvr_resolution_latency
.start_timer();

self.client
.post(api_url)
.json(&request_body)
.send()
.await
.map_err(|e| {
Error::MoveNameRegistry(MoveRegistryError::FailedToQueryExternalApi(
e.to_string(),
))
})?
};

if !res.status().is_success() {
return Err(Error::MoveNameRegistry(
Expand Down
24 changes: 24 additions & 0 deletions crates/sui-mvr-graphql-rpc/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const DB_QUERY_COST_BUCKETS: &[f64] = &[
pub(crate) struct Metrics {
pub db_metrics: Arc<DBMetrics>,
pub request_metrics: Arc<RequestMetrics>,
pub app_metrics: Arc<AppMetrics>,
}

#[derive(Clone)]
Expand Down Expand Up @@ -81,14 +82,23 @@ pub(crate) struct RequestMetrics {
pub inflight_requests: Gauge,
}

#[derive(Clone)]
pub(crate) struct AppMetrics {
/// The time it takes for the non-mainnet graphql service to resolve the mvr object from
/// mainnet.
pub external_mvr_resolution_latency: Histogram,
}

impl Metrics {
pub(crate) fn new(registry: &Registry) -> Self {
let db_metrics = DBMetrics::new(registry);
let request_metrics = RequestMetrics::new(registry);
let app_metrics = AppMetrics::new(registry);

Self {
db_metrics: Arc::new(db_metrics),
request_metrics: Arc::new(request_metrics),
app_metrics: Arc::new(app_metrics),
}
}

Expand Down Expand Up @@ -253,6 +263,20 @@ impl RequestMetrics {
}
}

impl AppMetrics {
pub(crate) fn new(registry: &Registry) -> Self {
Self {
external_mvr_resolution_latency: register_histogram_with_registry!(
"external_mvr_resolution_latency",
"The time it takes for the non-mainnet graphql service to resolve the mvr object from mainnet",
LATENCY_SEC_BUCKETS.to_vec(),
registry,
)
.unwrap(),
}
}
}

/// When an error occurs, GraphQL returns a vector of PathSegments,
/// that we can use to retrieve the last node which contains the error.
pub(crate) fn query_label_for_error(query: &[PathSegment]) -> String {
Expand Down
5 changes: 4 additions & 1 deletion crates/sui-mvr-graphql-rpc/src/server/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,10 @@ impl ServerBuilder {
.context_data(metrics.clone())
.context_data(config.clone())
.context_data(move_registry_config.clone())
.context_data(MoveRegistryDataLoader::new(move_registry_config));
.context_data(MoveRegistryDataLoader::new(
move_registry_config,
metrics.clone(),
));

if config.internal_features.feature_gate {
builder = builder.extension(FeatureGate);
Expand Down

0 comments on commit 851b6a6

Please sign in to comment.