[pdm-devel] [PATCH datacenter-manager 1/2] server: api: subscription: improve cache and update behavior

[pdm-devel] [PATCH datacenter-manager 1/2] server: api: subscription: improve cache and update behavior

[Dominik Csapak]

To avoid long load times in the ui, use only cached entries for
subscription info in the GET subscription api call by default, but add a
'force-load' parameter to restore the previous behavior.

The check_subscription api call now always load from all remotes to get
the most up-to-date information from them.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 server/src/api/nodes/subscription.rs             | 16 ++++++++++++----
 server/src/api/resources.rs                      |  5 ++++-
 .../proxmox-datacenter-manager-daily-update.rs   |  2 +-
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/server/src/api/nodes/subscription.rs b/server/src/api/nodes/subscription.rs
index c84487f0..0ad4c55c 100644
--- a/server/src/api/nodes/subscription.rs
+++ b/server/src/api/nodes/subscription.rs
@@ -30,12 +30,14 @@ fn apt_auth_file_opts() -> CreateOptions {
 }
 
 async fn get_all_subscription_infos(
+    max_age: u64,
+    cache_only: bool,
 ) -> Result<HashMap<String, (RemoteType, HashMap<String, Option<NodeSubscriptionInfo>>)>, Error> {
     let (remotes_config, _digest) = pdm_config::remotes::config()?;
 
     let mut subscription_info = HashMap::new();
     for (remote_name, remote) in remotes_config.iter() {
-        match get_subscription_info_for_remote(remote, 24 * 60 * 60).await {
+        match get_subscription_info_for_remote(remote, max_age, cache_only).await {
             Ok(info) => {
                 subscription_info.insert(remote_name.to_string(), (remote.ty, info));
             }
@@ -94,6 +96,12 @@ fn check_counts(stats: &SubscriptionStatistics) -> Result<(), Error> {
             node: {
                 schema: NODE_SCHEMA,
             },
+            "force-load": {
+                type: bool,
+                optional: true,
+                default: false,
+                description: "If true, tries to load subscription from remotes that are not cached.",
+            },
         },
     },
     returns: {
@@ -101,8 +109,8 @@ fn check_counts(stats: &SubscriptionStatistics) -> Result<(), Error> {
     }
 )]
 /// Return subscription status
-pub async fn get_subscription() -> Result<PdmSubscriptionInfo, Error> {
-    let infos = get_all_subscription_infos().await?;
+pub async fn get_subscription(force_load: bool) -> Result<PdmSubscriptionInfo, Error> {
+    let infos = get_all_subscription_infos(24 * 60 * 60, !force_load).await?;
 
     let statistics = count_subscriptions(&infos);
 
@@ -140,7 +148,7 @@ pub async fn get_subscription() -> Result<PdmSubscriptionInfo, Error> {
 )]
 /// Update subscription information
 pub async fn check_subscription() -> Result<(), Error> {
-    let infos = get_all_subscription_infos().await?;
+    let infos = get_all_subscription_infos(0, false).await?;
     let stats = count_subscriptions(&infos);
 
     if let Err(err) = check_counts(&stats) {
diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index c97644ad..7cbdb5b6 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -635,7 +635,7 @@ pub async fn get_subscription_status(
 
         let future = async move {
             let (node_status, error) =
-                match get_subscription_info_for_remote(&remote, max_age).await {
+                match get_subscription_info_for_remote(&remote, max_age, false).await {
                     Ok(mut node_status) => {
                         node_status.retain(|node, _| {
                             if let Some(view) = &view {
@@ -773,9 +773,12 @@ static SUBSCRIPTION_CACHE: LazyLock<RwLock<HashMap<String, CachedSubscriptionSta
 pub async fn get_subscription_info_for_remote(
     remote: &Remote,
     max_age: u64,
+    cache_only: bool,
 ) -> Result<HashMap<String, Option<NodeSubscriptionInfo>>, Error> {
     if let Some(cached_subscription) = get_cached_subscription_info(&remote.id, max_age) {
         Ok(cached_subscription.node_info)
+    } else if cache_only {
+        Ok(HashMap::new())
     } else {
         let node_info = fetch_remote_subscription_info(remote).await?;
         let now = proxmox_time::epoch_i64();
diff --git a/server/src/bin/proxmox-datacenter-manager-daily-update.rs b/server/src/bin/proxmox-datacenter-manager-daily-update.rs
index ad29c2c2..c2dd843c 100644
--- a/server/src/bin/proxmox-datacenter-manager-daily-update.rs
+++ b/server/src/bin/proxmox-datacenter-manager-daily-update.rs
@@ -26,7 +26,7 @@ async fn do_update(rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
     if let Err(err) = &api::nodes::subscription::check_subscription().await {
         log::error!("Error checking subscription - {err}");
     }
-    match api::nodes::subscription::get_subscription().await {
+    match api::nodes::subscription::get_subscription(true).await {
         Ok(info) if info.info.status == SubscriptionStatus::Active => {}
         Ok(info) => {
             log::warn!(
-- 
2.47.3



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel

[pdm-devel] [PATCH datacenter-manager 2/2] ui: subscription: use 'force-load' parameter

[Dominik Csapak]

on the first login, use 'force-load': true to let the backend refresh
all remotes whose information is older than 24 hours or never had any
info.

The remaining checks (e.g. refresh for updates, views, etc.) use the
(now) default caching behavior.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 ui/src/lib.rs             | 10 +++++++---
 ui/src/main.rs            |  2 +-
 ui/src/remotes/updates.rs |  2 +-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/ui/src/lib.rs b/ui/src/lib.rs
index 1aac7571..f9d4cbfd 100644
--- a/ui/src/lib.rs
+++ b/ui/src/lib.rs
@@ -9,7 +9,7 @@ use pwt::props::ContainerBuilder;
 use pwt::tr;
 use pwt::widget::{AlertDialog, Container};
 use serde::{Deserialize, Serialize};
-use serde_json::Value;
+use serde_json::{json, Value};
 
 use proxmox_yew_comp::http_get;
 
@@ -249,8 +249,12 @@ pub(crate) fn locale_compare(first: String, second: &str, numeric: bool) -> std:
 /// NOTE: This should be only used for PDM itself, or for when it's checked for a PDM specific
 /// feature, i.e., one that's not just relayed 1:1 to a specific remote node, as for that one should
 /// use the remote-specific check.
-pub async fn check_pdm_subscription() -> bool {
-    let data: Result<Value, _> = http_get("/nodes/localhost/subscription", None).await;
+pub async fn check_pdm_subscription(force: bool) -> bool {
+    let data: Result<Value, _> = http_get(
+        "/nodes/localhost/subscription",
+        Some(json!({"force-load": force})),
+    )
+    .await;
     let mut is_active = proxmox_yew_comp::subscription_is_active(Some(&data));
     if !is_active {
         if let Ok(Ok(info)) = data.map(serde_json::from_value::<PdmSubscriptionInfo>) {
diff --git a/ui/src/main.rs b/ui/src/main.rs
index 5f859dbc..a7a474b7 100644
--- a/ui/src/main.rs
+++ b/ui/src/main.rs
@@ -79,7 +79,7 @@ impl DatacenterManagerApp {
                     ctx.link().send_message(Msg::ConfirmSubscription);
                 } else {
                     self.async_pool.send_future(ctx.link().clone(), async move {
-                        let is_active = check_pdm_subscription().await;
+                        let is_active = check_pdm_subscription(true).await;
 
                         if !is_active {
                             Msg::ShowSubscriptionAlert
diff --git a/ui/src/remotes/updates.rs b/ui/src/remotes/updates.rs
index 12eaec87..d1d60910 100644
--- a/ui/src/remotes/updates.rs
+++ b/ui/src/remotes/updates.rs
@@ -394,7 +394,7 @@ impl LoadableComponent for UpdateTreeComponent {
 
                 link.clone().spawn(async move {
                     // Use the PDM subscription check for the global refresh all.
-                    let is_active = check_pdm_subscription().await;
+                    let is_active = check_pdm_subscription(false).await;
                     if !is_active {
                         link.change_view(Some(ViewState::ShowSubscriptionAlert));
                     } else {
-- 
2.47.3



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel

Re: [pdm-devel] [PATCH datacenter-manager 1/2] server: api: subscription: improve cache and update behavior

[Thomas Lamprecht]

Am 03.12.25 um 15:34 schrieb Dominik Csapak:
> To avoid long load times in the ui, use only cached entries for
> subscription info in the GET subscription api call by default, but add a
> 'force-load' parameter to restore the previous behavior.
> 
> The check_subscription api call now always load from all remotes to get
> the most up-to-date information from them.

hmm, this gets a bit confusing to me. Renaming the param to "cache-only"
would make that a bit better, as your name suggests that we always load
now, but with that param passed we still use the cache as long as the
age there is below max-age.

That said, I'm not sure this is the route I'd like to go at all, so just
lets not rush it and revisit this in a few days/weeks again.


What I'd probably would prefer is caching the unknown result and early
returning when that is recent enough.
Actually, it might be nice to have a global unknown (or offline) remote
(+ nodes?) cache, where we save when a remote was unknown and if it was
often in a certain time period we skip it from querying at all and short
circuit that to unknown in all places. This naturally needs to be fleshed
out and needs a bit of care, e.g., users should be able to see (and
manually clear) this state.


_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel