code cleanup

This commit is contained in:
Anastasia Lubennikova
2024-12-05 10:31:43 +00:00
parent 811e5675a3
commit 8d7fa802f4
3 changed files with 22 additions and 11 deletions

View File

@@ -311,7 +311,15 @@ async fn routes(req: Request<Body>, compute: &Arc<ComputeNode>) -> Response<Body
}
// handle HEAD method of /installed_extensions route
// just log info
// This is the signal from postgres that extension DDL occured and we need to update the metric.
//
// Don't wait for the result, because the caller doesn't need it
// just spawn a task and the metric will be updated eventually.
//
// In theory there could be multiple HEAD requests in a row, so we should protect
// from spawning multiple tasks, but in practice it's not a problem.
// TODO: add some mutex or quere?
// In practice, extensions are not installed very often, so it's not a problem
(&Method::HEAD, route) if route.starts_with("/installed_extensions") => {
info!("serving /installed_extensions HEAD request");
let status = compute.get_status();
@@ -326,12 +334,6 @@ async fn routes(req: Request<Body>, compute: &Arc<ComputeNode>) -> Response<Body
return resp;
}
// should I rewrite this to not wait for the result?
//let _ = crate::installed_extensions::get_installed_extensions(connstr).await;
// spwan a task to get installed extensions, but don't wait for the result
// since get_installed_extensions is a blocking call call it with spawn_blocking
// within an async task
let conf = compute.get_conn_conf(None);
task::spawn(async move {
let _ = task::spawn_blocking(move || {

View File

@@ -89,9 +89,14 @@ pub fn get_installed_extensions(mut conf: postgres::config::Config) -> Result<In
}
}
// reset the metric to handle dropped extensions and extension version changes
// reset the metric to handle dropped extensions and extension version changes -
// we need to remove them from the metric.
// It creates a race condition - if collector is called before we set the new values
// we will have a gap in the metrics. But it's not worth to optimize it further.
// we will have a gap in the metrics.
//
// TODO: Add a mutex to lock the metric update and collection
// so that the collector doesn't see intermediate state.
// Or is it ok for this metric to be eventually consistent?
INSTALLED_EXTENSIONS.reset();
for (key, ext) in extensions_map.iter() {
@@ -103,8 +108,7 @@ pub fn get_installed_extensions(mut conf: postgres::config::Config) -> Result<In
.set(n_databases);
}
// log the installed extensions
tracing::info!("Installed extensions: {:?}", extensions_map);
tracing::debug!("Installed extensions: {:?}", extensions_map);
Ok(InstalledExtensions {
extensions: extensions_map.into_values().collect(),
@@ -121,5 +125,6 @@ static INSTALLED_EXTENSIONS: Lazy<UIntGaugeVec> = Lazy::new(|| {
});
pub fn collect() -> Vec<MetricFamily> {
// TODO Add a mutex here to ensure that the metric is not updated while we are collecting it
INSTALLED_EXTENSIONS.collect()
}

View File

@@ -154,6 +154,10 @@ def test_installed_extensions(neon_simple_env: NeonEnv):
assert sample.value == 1
# WIP Test metric live update
# TODO: cleamup the test and stabilize it.
# Now there is a race, because there is a gap between the extension creation and the metric collection.
# This is fine for the real world, as we don't need to be 100% real time, but not convenient for the test.
def test_installed_extensions_metric_live_update(neon_simple_env: NeonEnv):
"""basic test for the endpoint that returns the list of installed extensions"""