Skip to content

Commit

Permalink
fix(payments_list): skip count query if no filters and add logging (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
apoorvdixit88 authored Oct 17, 2024
1 parent f3cd58b commit 584450f
Show file tree
Hide file tree
Showing 9 changed files with 85 additions and 60 deletions.
11 changes: 11 additions & 0 deletions crates/api_models/src/payments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4573,6 +4573,17 @@ pub struct PaymentListFilterConstraints {
/// The List of all the card networks to filter payments list
pub card_network: Option<Vec<enums::CardNetwork>>,
}

impl PaymentListFilterConstraints {
pub fn has_no_attempt_filters(&self) -> bool {
self.connector.is_none()
&& self.payment_method.is_none()
&& self.payment_method_type.is_none()
&& self.authentication_type.is_none()
&& self.merchant_connector_id.is_none()
}
}

#[derive(Clone, Debug, serde::Serialize)]
pub struct PaymentListFilters {
/// The list of available connector filters
Expand Down
17 changes: 11 additions & 6 deletions crates/diesel_models/src/query/payment_attempt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,6 @@ impl PaymentAttempt {
payment_method: Option<Vec<enums::PaymentMethod>>,
payment_method_type: Option<Vec<enums::PaymentMethodType>>,
authentication_type: Option<Vec<enums::AuthenticationType>>,
profile_id_list: Option<Vec<common_utils::id_type::ProfileId>>,
merchant_connector_id: Option<Vec<common_utils::id_type::MerchantConnectorAccountId>>,
) -> StorageResult<i64> {
let mut filter = <Self as HasTable>::table()
Expand All @@ -394,17 +393,23 @@ impl PaymentAttempt {
if let Some(merchant_connector_id) = merchant_connector_id {
filter = filter.filter(dsl::merchant_connector_id.eq_any(merchant_connector_id))
}
if let Some(profile_id_list) = profile_id_list {
filter = filter.filter(dsl::profile_id.eq_any(profile_id_list))
}

router_env::logger::debug!(query = %debug_query::<Pg, _>(&filter).to_string());

db_metrics::track_database_call::<<Self as HasTable>::Table, _, _>(
// TODO: Remove these logs after debugging the issue for delay in count query
let start_time = std::time::Instant::now();
router_env::logger::debug!("Executing count query start_time: {:?}", start_time);
let result = db_metrics::track_database_call::<<Self as HasTable>::Table, _, _>(
filter.get_result_async::<i64>(conn),
db_metrics::DatabaseOperation::Filter,
)
.await
.change_context(DatabaseError::Others)
.attach_printable("Error filtering count of payments")
.attach_printable("Error filtering count of payments");

let duration = start_time.elapsed();
router_env::logger::debug!("Completed count query in {:?}", duration);

result
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ pub trait PaymentAttemptInterface {
payment_method_type: Option<Vec<storage_enums::PaymentMethodType>>,
authentication_type: Option<Vec<storage_enums::AuthenticationType>>,
merchant_connector_id: Option<Vec<id_type::MerchantConnectorAccountId>>,
profile_id_list: Option<Vec<id_type::ProfileId>>,
storage_scheme: storage_enums::MerchantStorageScheme,
) -> error_stack::Result<i64, errors::StorageError>;
}
Expand Down
105 changes: 60 additions & 45 deletions crates/router/src/core/payments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3847,54 +3847,69 @@ pub async fn apply_filters_on_payments(
merchant_key_store: domain::MerchantKeyStore,
constraints: api::PaymentListFilterConstraints,
) -> RouterResponse<api::PaymentListResponseV2> {
let limit = &constraints.limit;
helpers::validate_payment_list_request_for_joins(*limit)?;
let db: &dyn StorageInterface = state.store.as_ref();
let pi_fetch_constraints = (constraints.clone(), profile_id_list.clone()).try_into()?;
let list: Vec<(storage::PaymentIntent, storage::PaymentAttempt)> = db
.get_filtered_payment_intents_attempt(
&(&state).into(),
merchant.get_id(),
&pi_fetch_constraints,
&merchant_key_store,
merchant.storage_scheme,
)
.await
.to_not_found_response(errors::ApiErrorResponse::PaymentNotFound)?;
let data: Vec<api::PaymentsResponse> =
list.into_iter().map(ForeignFrom::foreign_from).collect();

let active_attempt_ids = db
.get_filtered_active_attempt_ids_for_total_count(
merchant.get_id(),
&pi_fetch_constraints,
merchant.storage_scheme,
)
.await
.to_not_found_response(errors::ApiErrorResponse::InternalServerError)?;
common_utils::metrics::utils::record_operation_time(
async {
let limit = &constraints.limit;
helpers::validate_payment_list_request_for_joins(*limit)?;
let db: &dyn StorageInterface = state.store.as_ref();
let pi_fetch_constraints = (constraints.clone(), profile_id_list.clone()).try_into()?;
let list: Vec<(storage::PaymentIntent, storage::PaymentAttempt)> = db
.get_filtered_payment_intents_attempt(
&(&state).into(),
merchant.get_id(),
&pi_fetch_constraints,
&merchant_key_store,
merchant.storage_scheme,
)
.await
.to_not_found_response(errors::ApiErrorResponse::PaymentNotFound)?;
let data: Vec<api::PaymentsResponse> =
list.into_iter().map(ForeignFrom::foreign_from).collect();

let active_attempt_ids = db
.get_filtered_active_attempt_ids_for_total_count(
merchant.get_id(),
&pi_fetch_constraints,
merchant.storage_scheme,
)
.await
.to_not_found_response(errors::ApiErrorResponse::InternalServerError)?;

let total_count = db
.get_total_count_of_filtered_payment_attempts(
merchant.get_id(),
&active_attempt_ids,
constraints.connector,
constraints.payment_method,
constraints.payment_method_type,
constraints.authentication_type,
constraints.merchant_connector_id,
pi_fetch_constraints.get_profile_id_list(),
merchant.storage_scheme,
)
.await
.change_context(errors::ApiErrorResponse::InternalServerError)?;
let total_count = if constraints.has_no_attempt_filters() {
i64::try_from(active_attempt_ids.len())
.change_context(errors::ApiErrorResponse::InternalServerError)
.attach_printable("Error while converting from usize to i64")
} else {
db.get_total_count_of_filtered_payment_attempts(
merchant.get_id(),
&active_attempt_ids,
constraints.connector,
constraints.payment_method,
constraints.payment_method_type,
constraints.authentication_type,
constraints.merchant_connector_id,
merchant.storage_scheme,
)
.await
.change_context(errors::ApiErrorResponse::InternalServerError)
}?;

Ok(services::ApplicationResponse::Json(
api::PaymentListResponseV2 {
count: data.len(),
total_count,
data,
Ok(services::ApplicationResponse::Json(
api::PaymentListResponseV2 {
count: data.len(),
total_count,
data,
},
))
},
))
&metrics::PAYMENT_LIST_LATENCY,
&metrics::CONTEXT,
&[router_env::opentelemetry::KeyValue::new(
"merchant_id",
merchant.get_id().clone(),
)],
)
.await
}

#[cfg(all(feature = "olap", feature = "v1"))]
Expand Down
2 changes: 0 additions & 2 deletions crates/router/src/db/kafka_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1601,7 +1601,6 @@ impl PaymentAttemptInterface for KafkaStore {
payment_method_type: Option<Vec<common_enums::PaymentMethodType>>,
authentication_type: Option<Vec<common_enums::AuthenticationType>>,
merchant_connector_id: Option<Vec<id_type::MerchantConnectorAccountId>>,
profile_id_list: Option<Vec<id_type::ProfileId>>,
storage_scheme: MerchantStorageScheme,
) -> CustomResult<i64, errors::DataStorageError> {
self.diesel_store
Expand All @@ -1613,7 +1612,6 @@ impl PaymentAttemptInterface for KafkaStore {
payment_method_type,
authentication_type,
merchant_connector_id,
profile_id_list,
storage_scheme,
)
.await
Expand Down
2 changes: 2 additions & 0 deletions crates/router/src/routes/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ counter_metric!(PAYMENT_OPS_COUNT, GLOBAL_METER);

counter_metric!(PAYMENT_COUNT, GLOBAL_METER);
counter_metric!(SUCCESSFUL_PAYMENT, GLOBAL_METER);
//TODO: This can be removed, added for payment list debugging
histogram_metric!(PAYMENT_LIST_LATENCY, GLOBAL_METER);

counter_metric!(REFUND_COUNT, GLOBAL_METER);
counter_metric!(SUCCESSFUL_REFUND, GLOBAL_METER);
Expand Down
2 changes: 1 addition & 1 deletion crates/router/src/routes/payments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1091,7 +1091,7 @@ pub async fn profile_payments_list_by_filter(
|state, auth: auth::AuthenticationData, req, _| {
payments::apply_filters_on_payments(
state,
auth.merchant_account,
auth.merchant_account.clone(),
auth.profile_id.map(|profile_id| vec![profile_id]),
auth.key_store,
req,
Expand Down
1 change: 0 additions & 1 deletion crates/storage_impl/src/mock_db/payment_attempt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ impl PaymentAttemptInterface for MockDb {
_payment_method_type: Option<Vec<common_enums::PaymentMethodType>>,
_authentication_type: Option<Vec<common_enums::AuthenticationType>>,
_merchanat_connector_id: Option<Vec<common_utils::id_type::MerchantConnectorAccountId>>,
_profile_id_list: Option<Vec<common_utils::id_type::ProfileId>>,
_storage_scheme: storage_enums::MerchantStorageScheme,
) -> CustomResult<i64, StorageError> {
Err(StorageError::MockDbError)?
Expand Down
4 changes: 0 additions & 4 deletions crates/storage_impl/src/payments/payment_attempt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,6 @@ impl<T: DatabaseStore> PaymentAttemptInterface for RouterStore<T> {
payment_method_type: Option<Vec<common_enums::PaymentMethodType>>,
authentication_type: Option<Vec<common_enums::AuthenticationType>>,
merchant_connector_id: Option<Vec<common_utils::id_type::MerchantConnectorAccountId>>,
profile_id_list: Option<Vec<common_utils::id_type::ProfileId>>,
_storage_scheme: MerchantStorageScheme,
) -> CustomResult<i64, errors::StorageError> {
let conn = self
Expand All @@ -425,7 +424,6 @@ impl<T: DatabaseStore> PaymentAttemptInterface for RouterStore<T> {
payment_method,
payment_method_type,
authentication_type,
profile_id_list,
merchant_connector_id,
)
.await
Expand Down Expand Up @@ -1280,7 +1278,6 @@ impl<T: DatabaseStore> PaymentAttemptInterface for KVRouterStore<T> {
payment_method_type: Option<Vec<common_enums::PaymentMethodType>>,
authentication_type: Option<Vec<common_enums::AuthenticationType>>,
merchant_connector_id: Option<Vec<common_utils::id_type::MerchantConnectorAccountId>>,
profile_id_list: Option<Vec<common_utils::id_type::ProfileId>>,
storage_scheme: MerchantStorageScheme,
) -> CustomResult<i64, errors::StorageError> {
self.router_store
Expand All @@ -1292,7 +1289,6 @@ impl<T: DatabaseStore> PaymentAttemptInterface for KVRouterStore<T> {
payment_method_type,
authentication_type,
merchant_connector_id,
profile_id_list,
storage_scheme,
)
.await
Expand Down

0 comments on commit 584450f

Please sign in to comment.