From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 77B12D300 for ; Wed, 20 Sep 2023 18:04:21 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 58413185AC for ; Wed, 20 Sep 2023 18:04:21 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 20 Sep 2023 18:04:19 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id AB6A24843B for ; Wed, 20 Sep 2023 18:04:19 +0200 (CEST) Message-ID: Date: Wed, 20 Sep 2023 18:04:17 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.0 To: Proxmox Backup Server development discussion , Stefan Lendl References: <20230913141418.237749-1-g.goller@proxmox.com> <91615123.4069.1695220756800@webmail.proxmox.com> Content-Language: en-US From: Gabriel Goller In-Reply-To: <91615123.4069.1695220756800@webmail.proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.037 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_ASCII_DIVIDERS 0.8 Email that uses ascii formatting dividers and possible spam tricks KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -1.473 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com, datastore.rs, rec.id, record.data] Subject: Re: [pbs-devel] [PATCH proxmox-backup] close #4723: updated gc view in the ui X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Sep 2023 16:04:21 -0000 Thanks for the review! (comments inline) On 9/20/23 16:39, Stefan Lendl wrote: > I tried your patch on a fresh PBS install. You are not considering the case where GC was never executed. > Some places annotated below. > > In case GC never ran, or is not scheduled you're still displaying the entire table in the GUI. > > >> Gabriel Goller hat am 13.09.2023 16:14 CEST geschrieben: >> >> >> Updated the GC overview in the datastore page. This enables >> us to see: >> - schedule >> - State (of last run) >> - Duration (of last run) >> - Last Run >> - Next Run >> - Pending Chunks (of last run) >> - Removed Chunks (of last run) >> >> Added `ObjectGrid` for GCView, moved to different file. Added >> endpoint `gc_info` that returns all the necessary config and >> last run stats. >> >> Signed-off-by: Gabriel Goller >> --- >> pbs-api-types/src/datastore.rs | 30 ++++++++ >> src/api2/admin/datastore.rs | 90 ++++++++++++++++++++--- >> www/Makefile | 1 + >> www/Utils.js | 12 ++-- >> www/config/GCView.js | 128 +++++++++++++++++++++++++++++++++ >> www/datastore/PruneAndGC.js | 93 ++---------------------- >> 6 files changed, 252 insertions(+), 102 deletions(-) >> mode change 100644 => 100755 src/api2/admin/datastore.rs >> create mode 100644 www/config/GCView.js >> >> diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs >> index 73c4890e..5d6c8729 100644 >> --- a/pbs-api-types/src/datastore.rs >> +++ b/pbs-api-types/src/datastore.rs >> @@ -1250,6 +1250,36 @@ pub struct GarbageCollectionStatus { >> pub still_bad: usize, >> } >> >> +#[api( >> + properties: { >> + "last-run-upid": { >> + optional: true, >> + type: UPID, >> + }, >> + }, >> +)] >> +#[derive(Clone, Default, Serialize, Deserialize, PartialEq)] >> +#[serde(rename_all = "kebab-case")] >> +/// Garbage Collection general info >> +pub struct GarbageCollectionInfo { >> + /// upid of the last run gc job >> + pub last_run_upid: Option, >> + /// Number of removed chunks >> + pub removed_chunks: usize, >> + /// Number of pending chunks >> + pub pending_chunks: usize, >> + /// Schedule of the gc job >> + pub schedule: Option, >> + /// Time of the next gc run >> + pub next_run: Option, >> + /// Endtime of the last gc run >> + pub last_run_endtime: Option, >> + /// State of the last gc run >> + pub last_run_state: Option, >> + /// Duration of last gc run >> + pub duration: Option, >> +} >> + >> #[api( >> properties: { >> "gc-status": { >> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs >> old mode 100644 >> new mode 100755 >> index a95031e7..9218f39a >> --- a/src/api2/admin/datastore.rs >> +++ b/src/api2/admin/datastore.rs >> @@ -10,6 +10,7 @@ use anyhow::{bail, format_err, Error}; >> use futures::*; >> use hyper::http::request::Parts; >> use hyper::{header, Body, Response, StatusCode}; >> +use proxmox_time::CalendarEvent; >> use serde::Deserialize; >> use serde_json::{json, Value}; >> use tokio_stream::wrappers::ReceiverStream; >> @@ -33,13 +34,14 @@ use pxar::EntryKind; >> >> use pbs_api_types::{ >> print_ns_and_snapshot, print_store_and_ns, Authid, BackupContent, BackupNamespace, BackupType, >> - Counts, CryptMode, DataStoreListItem, DataStoreStatus, GarbageCollectionStatus, GroupListItem, >> - KeepOptions, Operation, PruneJobOptions, RRDMode, RRDTimeFrame, SnapshotListItem, >> - SnapshotVerifyState, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, >> - BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, DATASTORE_SCHEMA, IGNORE_VERIFIED_BACKUPS_SCHEMA, >> - MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, >> - PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, >> - UPID_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA, >> + Counts, CryptMode, DataStoreConfig, DataStoreListItem, DataStoreStatus, GarbageCollectionInfo, >> + GarbageCollectionStatus, GroupListItem, JobScheduleStatus, KeepOptions, Operation, >> + PruneJobOptions, RRDMode, RRDTimeFrame, SnapshotListItem, SnapshotVerifyState, >> + BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA, >> + BACKUP_TYPE_SCHEMA, DATASTORE_SCHEMA, IGNORE_VERIFIED_BACKUPS_SCHEMA, MAX_NAMESPACE_DEPTH, >> + NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, >> + PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, UPID, UPID_SCHEMA, >> + VERIFICATION_OUTDATED_AFTER_SCHEMA, >> }; >> use pbs_client::pxar::{create_tar, create_zip}; >> use pbs_config::CachedUserInfo; >> @@ -67,7 +69,7 @@ use crate::backup::{ >> ListAccessibleBackupGroups, NS_PRIVS_OK, >> }; >> >> -use crate::server::jobstate::Job; >> +use crate::server::jobstate::{compute_schedule_status, Job, JobState}; >> >> const GROUP_NOTES_FILE_NAME: &str = "notes"; >> >> @@ -1199,6 +1201,74 @@ pub fn garbage_collection_status( >> Ok(status) >> } >> >> +#[api( >> + input: { >> + properties: { >> + store: { >> + schema: DATASTORE_SCHEMA, >> + }, >> + }, >> + }, >> + returns: { >> + type: GarbageCollectionInfo, >> + }, >> + access: { >> + permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false), >> + }, >> +)] >> +/// Garbage collection status. >> +pub fn garbage_collection_info( >> + store: String, >> + _info: &ApiMethod, >> + _rpcenv: &mut dyn RpcEnvironment, >> +) -> Result { >> + let (config, _) = pbs_config::datastore::config()?; >> + let store_config: DataStoreConfig = config.lookup("datastore", &store)?; >> + >> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?; >> + let status = datastore.last_gc_status(); >> + >> + let upid = status.upid.unwrap(); > > On a fresh PBS install without any previous GC runs, status.upid will be None which causes the server to crash. > > Sep 20 13:53:19 pbs-devel proxmox-backup-proxy[441]: thread 'tokio-runtime-worker' panicked at 'called `Option::unwrap()` on a `None` value', src/api2/admin/datastore.rs:1234:28 Right, I don't know how I missed that :| >> + let last_state = JobState::load("garbage_collection", &store) >> + .map_err(|err| format_err!("could not open statefile for {}: {}", upid, err)) >> + .ok(); >> + >> + let mut computed_schedule: JobScheduleStatus = JobScheduleStatus::default(); >> + if let Some(state) = last_state { >> + computed_schedule = compute_schedule_status(&state, Some(&upid)).unwrap(); >> + } >> + >> + // calculate next event >> + if let Some(schedule) = &store_config.gc_schedule { >> + if let (Ok(event), Some(last_run)) = ( >> + schedule.parse::(), >> + computed_schedule.last_run_endtime, >> + ) { >> + if let Ok(next_event) = event.compute_next_event(last_run) { >> + computed_schedule.next_run = next_event; >> + } >> + } >> + } >> + >> + let mut duration = None; >> + if let (Ok(upid), Some(endtime)) = (upid.parse::(), computed_schedule.last_run_endtime) { >> + duration = Some(endtime - upid.starttime); >> + } > duration will be None if it never ran. -> The GUI displays <0.1s > Added `#[serde(skip_serializing_if = "Option::is_none")]` to the field, because the frontend `Proxmox.util.render_*` functions don't handle null. >> + >> + let info = GarbageCollectionInfo { >> + last_run_upid: Some(upid), >> + removed_chunks: status.removed_chunks, >> + pending_chunks: status.pending_chunks, >> + schedule: store_config.gc_schedule, >> + next_run: computed_schedule.next_run, >> + last_run_endtime: computed_schedule.last_run_endtime, >> + last_run_state: computed_schedule.last_run_state, >> + duration, >> + }; >> + >> + Ok(info) >> +} >> + >> #[api( >> returns: { >> description: "List the accessible datastores.", >> @@ -2265,6 +2335,10 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[ >> .get(&API_METHOD_GARBAGE_COLLECTION_STATUS) >> .post(&API_METHOD_START_GARBAGE_COLLECTION), >> ), >> + ( >> + "gc_info", >> + &Router::new().get(&API_METHOD_GARBAGE_COLLECTION_INFO), >> + ), >> ( >> "group-notes", >> &Router::new() >> diff --git a/www/Makefile b/www/Makefile >> index 04c12b31..b67fd73f 100644 >> --- a/www/Makefile >> +++ b/www/Makefile >> @@ -62,6 +62,7 @@ JSSRC= \ >> config/SyncView.js \ >> config/VerifyView.js \ >> config/PruneView.js \ >> + config/GCView.js \ >> config/WebauthnView.js \ >> config/CertificateView.js \ >> config/NodeOptionView.js \ >> diff --git a/www/Utils.js b/www/Utils.js >> index 2eca600e..66293b17 100644 >> --- a/www/Utils.js >> +++ b/www/Utils.js >> @@ -199,14 +199,14 @@ Ext.define('PBS.Utils', { >> return fingerprint.substring(0, 23); >> }, >> >> - render_task_status: function(value, metadata, record) { >> - if (!record.data['last-run-upid']) { >> - return '-'; >> + render_task_status: function(value, metadata, record, rowIndex, colIndex, store) { >> + if (!record.data['last-run-upid'] && !store.getById('last-run-upid')?.data.value) { >> + return '-'; >> } >> >> - if (!record.data['last-run-endtime']) { >> - metadata.tdCls = 'x-grid-row-loading'; >> - return ''; >> + if (!record.data['last-run-endtime'] && !store.getById('last-run-endtime')?.data.value) { >> + metadata.tdCls = 'x-grid-row-loading'; >> + return ''; >> } >> >> let parsed = Proxmox.Utils.parse_task_status(value); >> diff --git a/www/config/GCView.js b/www/config/GCView.js >> new file mode 100644 >> index 00000000..70cd114c >> --- /dev/null >> +++ b/www/config/GCView.js >> @@ -0,0 +1,128 @@ >> +Ext.define('PBS.Datastore.GCOptions', { >> + extend: 'Proxmox.grid.ObjectGrid', >> + alias: 'widget.pbsGCJobView', >> + mixins: ['Proxmox.Mixin.CBind'], >> + >> + onlineHelp: 'maintenance_pruning', >> + >> + cbindData: function(initial) { >> + let me = this; >> + >> + me.datastore = encodeURIComponent(me.datastore); >> + me.url = `/api2/json/admin/datastore/${me.datastore}/gc_info`; >> + me.editorConfig = { >> + url: `/api2/extjs/config/datastore/${me.datastore}`, >> + }; >> + return {}; >> + }, >> + >> + controller: { >> + xclass: 'Ext.app.ViewController', >> + >> + edit: function() { >> + this.getView().run_editor(); >> + }, >> + >> + garbageCollect: function() { >> + let me = this; >> + let view = me.getView(); >> + Proxmox.Utils.API2Request({ >> + url: `/admin/datastore/${view.datastore}/gc`, >> + method: 'POST', >> + failure: function(response) { >> + Ext.Msg.alert(gettext('Error'), response.htmlStatus); >> + }, >> + success: function(response, options) { >> + Ext.create('Proxmox.window.TaskViewer', { >> + upid: response.result.data, >> + }).show(); >> + }, >> + }); >> + }, >> + }, >> + >> + tbar: [ >> + { >> + xtype: 'proxmoxButton', >> + text: gettext('Edit'), >> + enableFn: (rec) => rec.id === 'schedule', >> + disabled: true, >> + handler: 'edit', >> + }, >> + '-', >> + { >> + xtype: 'proxmoxButton', >> + text: gettext('Start Garbage Collection'), >> + selModel: null, >> + handler: 'garbageCollect', >> + }, >> + ], >> + >> + listeners: { >> + activate: function() { this.rstore.startUpdate(); }, >> + destroy: function() { this.rstore.stopUpdate(); }, >> + deactivate: function() { this.rstore.stopUpdate(); }, >> + itemdblclick: 'edit', >> + }, >> + >> + rows: { >> + "schedule": { >> + required: true, >> + defaultValue: Proxmox.Utils.NoneText, >> + header: gettext('Schedule'), >> + editor: { >> + xtype: 'proxmoxWindowEdit', >> + title: gettext('GC Schedule'), >> + onlineHelp: 'maintenance_gc', >> + items: { >> + xtype: 'pbsCalendarEvent', >> + name: 'gc-schedule', >> + fieldLabel: gettext("GC Schedule"), >> + emptyText: Proxmox.Utils.noneText, >> + deleteEmpty: true, >> + }, >> + }, >> + }, >> + "last-run-state": { >> + required: true, >> + defaultValue: Proxmox.Utils.NoneText, >> + header: gettext('State'), >> + renderer: PBS.Utils.render_task_status, >> + disabled: true, >> + }, >> + "duration": { >> + required: true, >> + defaultValue: Proxmox.Utils.NoneText, >> + header: gettext('Duration'), >> + renderer: Proxmox.Utils.render_duration, >> + disabled: true, >> + }, >> + "last-run-endtime": { >> + required: true, >> + defaultValue: Proxmox.Utils.NoneText, >> + header: gettext('Last Run'), >> + renderer: PBS.Utils.render_optional_timestamp, >> + disabled: true, >> + }, >> + "next-run": { >> + required: true, >> + defaultValue: Proxmox.Utils.NoneText, >> + header: gettext('Next Run'), >> + renderer: PBS.Utils.render_next_task_run, >> + disabled: true, >> + }, >> + "pending-chunks": { >> + required: true, >> + defaultValue: Proxmox.Utils.NoneText, >> + header: gettext('Pending Chunks'), >> + disabled: true, >> + }, >> + "removed-chunks": { >> + required: true, >> + defaultValue: Proxmox.Utils.NoneText, >> + header: gettext('Removed Chunks'), >> + disabled: true, >> + }, >> + "last-run-upid": { visible: false }, >> + }, >> +}); >> diff --git a/www/datastore/PruneAndGC.js b/www/datastore/PruneAndGC.js >> index aab98dad..c80facfb 100644 >> --- a/www/datastore/PruneAndGC.js >> +++ b/www/datastore/PruneAndGC.js >> @@ -1,88 +1,3 @@ >> -Ext.define('PBS.Datastore.GCOptions', { >> - extend: 'Proxmox.grid.ObjectGrid', >> - alias: 'widget.pbsDatastoreGCOpts', >> - mixins: ['Proxmox.Mixin.CBind'], >> - >> - onlineHelp: 'maintenance_pruning', >> - >> - cbindData: function(initial) { >> - let me = this; >> - >> - me.datastore = encodeURIComponent(me.datastore); >> - me.url = `/api2/json/config/datastore/${me.datastore}`; >> - me.editorConfig = { >> - url: `/api2/extjs/config/datastore/${me.datastore}`, >> - }; >> - return {}; >> - }, >> - >> - controller: { >> - xclass: 'Ext.app.ViewController', >> - >> - edit: function() { this.getView().run_editor(); }, >> - >> - garbageCollect: function() { >> - let me = this; >> - let view = me.getView(); >> - Proxmox.Utils.API2Request({ >> - url: `/admin/datastore/${view.datastore}/gc`, >> - method: 'POST', >> - failure: function(response) { >> - Ext.Msg.alert(gettext('Error'), response.htmlStatus); >> - }, >> - success: function(response, options) { >> - Ext.create('Proxmox.window.TaskViewer', { >> - upid: response.result.data, >> - }).show(); >> - }, >> - }); >> - }, >> - }, >> - >> - tbar: [ >> - { >> - xtype: 'proxmoxButton', >> - text: gettext('Edit'), >> - disabled: true, >> - handler: 'edit', >> - }, >> - '-', >> - { >> - xtype: 'proxmoxButton', >> - text: gettext('Start Garbage Collection'), >> - selModel: null, >> - handler: 'garbageCollect', >> - }, >> - ], >> - >> - listeners: { >> - activate: function() { this.rstore.startUpdate(); }, >> - destroy: function() { this.rstore.stopUpdate(); }, >> - deactivate: function() { this.rstore.stopUpdate(); }, >> - itemdblclick: 'edit', >> - }, >> - >> - rows: { >> - "gc-schedule": { >> - required: true, >> - defaultValue: Proxmox.Utils.NoneText, >> - header: gettext('Garbage Collection Schedule'), >> - editor: { >> - xtype: 'proxmoxWindowEdit', >> - title: gettext('GC Schedule'), >> - onlineHelp: 'maintenance_gc', >> - items: { >> - xtype: 'pbsCalendarEvent', >> - name: 'gc-schedule', >> - fieldLabel: gettext("GC Schedule"), >> - emptyText: Proxmox.Utils.noneText, >> - deleteEmpty: true, >> - }, >> - }, >> - }, >> - }, >> -}); >> - >> Ext.define('PBS.Datastore.PruneAndGC', { >> extend: 'Ext.panel.Panel', >> alias: 'widget.pbsDatastorePruneAndGC', >> @@ -99,10 +14,12 @@ Ext.define('PBS.Datastore.PruneAndGC', { >> }, >> items: [ >> { >> - xtype: 'pbsDatastoreGCOpts', >> + xtype: 'pbsGCJobView', >> title: gettext('Garbage Collection'), >> - itemId: 'datastore-gc', >> + itemId: 'datastore-gc-jobs', >> nodename: 'localhost', >> + flex: 1, >> + minHeight: 50, > > even though you have a small minHeight specified here, the div has a rather large empty space at the bottom. > zooming out also does not look good.. Removed the flex and minHeight properties, should look better now. Submitted a v2! >> cbind: { >> datastore: '{datastore}', >> }, >> @@ -111,7 +28,7 @@ Ext.define('PBS.Datastore.PruneAndGC', { >> xtype: 'pbsPruneJobView', >> nodename: 'localhost', >> itemId: 'datastore-prune-jobs', >> - flex: 1, >> + flex: 3, >> minHeight: 200, >> cbind: { >> datastore: '{datastore}', >> -- >> 2.39.2 >> >> >> >> _______________________________________________ >> pbs-devel mailing list >> pbs-devel@lists.proxmox.com >> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel > >