From: Stefan Lendl <s.lendl@proxmox.com>
To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox-backup] close #4723: updated gc view in the ui
Date: Wed, 20 Sep 2023 16:39:16 +0200 (CEST) [thread overview]
Message-ID: <91615123.4069.1695220756800@webmail.proxmox.com> (raw)
In-Reply-To: <20230913141418.237749-1-g.goller@proxmox.com>
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 <g.goller@proxmox.com> 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 <g.goller@proxmox.com>
> ---
> 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<String>,
> + /// Number of removed chunks
> + pub removed_chunks: usize,
> + /// Number of pending chunks
> + pub pending_chunks: usize,
> + /// Schedule of the gc job
> + pub schedule: Option<String>,
> + /// Time of the next gc run
> + pub next_run: Option<i64>,
> + /// Endtime of the last gc run
> + pub last_run_endtime: Option<i64>,
> + /// State of the last gc run
> + pub last_run_state: Option<String>,
> + /// Duration of last gc run
> + pub duration: Option<i64>,
> +}
> +
> #[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<GarbageCollectionInfo, Error> {
> + 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
> + 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::<CalendarEvent>(),
> + 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::<UPID>(), computed_schedule.last_run_endtime) {
> + duration = Some(endtime - upid.starttime);
> + }
duration will be None if it never ran. -> The GUI displays <0.1s
> +
> + 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..
> 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
next prev parent reply other threads:[~2023-09-20 14:39 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-13 14:14 Gabriel Goller
2023-09-20 14:39 ` Stefan Lendl [this message]
2023-09-20 16:04 ` Gabriel Goller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=91615123.4069.1695220756800@webmail.proxmox.com \
--to=s.lendl@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox