Skip to content

Commit

Permalink
proxy: exclude triple logging of connect compute errors (#9277)
Browse files Browse the repository at this point in the history
Fixes (#9020)
 - Use the compute::COULD_NOT_CONNECT for connection error message;
 - Eliminate logging for one connection attempt;
 - Typo fix.
  • Loading branch information
awarus authored Oct 4, 2024
1 parent 6c05f89 commit 2d248ae
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 7 deletions.
2 changes: 1 addition & 1 deletion proxy/src/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use tokio_postgres::tls::MakeTlsConnect;
use tokio_postgres_rustls::MakeRustlsConnect;
use tracing::{error, info, warn};

const COULD_NOT_CONNECT: &str = "Couldn't connect to compute node";
pub const COULD_NOT_CONNECT: &str = "Couldn't connect to compute node";

#[derive(Debug, Error)]
pub(crate) enum ConnectionError {
Expand Down
12 changes: 6 additions & 6 deletions proxy/src/proxy/connect_compute.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{
auth::backend::ComputeCredentialKeys,
compute::COULD_NOT_CONNECT,
compute::{self, PostgresConnection},
config::RetryConfig,
console::{self, errors::WakeComputeError, locks::ApiLocks, CachedNodeInfo, NodeInfo},
Expand All @@ -15,7 +16,7 @@ use crate::{
use async_trait::async_trait;
use pq_proto::StartupMessageParams;
use tokio::time;
use tracing::{error, info, warn};
use tracing::{debug, info, warn};

use super::retry::ShouldRetryWakeCompute;

Expand Down Expand Up @@ -116,7 +117,6 @@ where

node_info.set_keys(user_info.get_keys());
node_info.allow_self_signed_compute = allow_self_signed_compute;
// let mut node_info = credentials.get_node_info(ctx, user_info).await?;
mechanism.update_connect_config(&mut node_info.config);
let retry_type = RetryType::ConnectToCompute;

Expand All @@ -139,10 +139,10 @@ where
Err(e) => e,
};

error!(error = ?err, "could not connect to compute node");
debug!(error = ?err, COULD_NOT_CONNECT);

let node_info = if !node_info.cached() || !err.should_retry_wake_compute() {
// If we just recieved this from cplane and dodn't get it from cache, we shouldn't retry.
// If we just recieved this from cplane and didn't get it from cache, we shouldn't retry.
// Do not need to retrieve a new node_info, just return the old one.
if should_retry(&err, num_retries, connect_to_compute_retry_config) {
Metrics::get().proxy.retries_metric.observe(
Expand Down Expand Up @@ -191,7 +191,7 @@ where
}
Err(e) => {
if !should_retry(&e, num_retries, connect_to_compute_retry_config) {
error!(error = ?e, num_retries, retriable = false, "couldn't connect to compute node");
// Don't log an error here, caller will print the error
Metrics::get().proxy.retries_metric.observe(
RetriesMetricGroup {
outcome: ConnectOutcome::Failed,
Expand All @@ -202,7 +202,7 @@ where
return Err(e.into());
}

warn!(error = ?e, num_retries, retriable = true, "couldn't connect to compute node");
warn!(error = ?e, num_retries, retriable = true, COULD_NOT_CONNECT);
}
};

Expand Down

1 comment on commit 2d248ae

@github-actions
Copy link

Choose a reason for hiding this comment

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

5173 tests run: 4956 passed, 0 failed, 217 skipped (full report)


Flaky tests (3)

Postgres 16

Postgres 14

Code coverage* (full report)

  • functions: 31.4% (7508 of 23942 functions)
  • lines: 49.6% (60277 of 121590 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
2d248ae at 2024-10-04T17:45:41.656Z :recycle:

Please sign in to comment.