[pbs-devel] [PATCH proxmox-backup] close #4723: updated gc view in the ui
Gabriel Goller
g.goller at proxmox.com
Wed Sep 20 18:04:17 CEST 2023
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 <g.goller at 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 at 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
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::<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
>
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 at lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
More information about the pbs-devel
mailing list