View Issue Details

IDProjectCategoryView StatusLast Update
0000282bareos-core[All Projects] directorpublic2015-03-31 15:25
ReporterjkhradilAssigned To 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionreopened 
PlatformLinuxOSRHELOS Version6
Product Version13.2.2 
Fixed in Version 
Summary0000282: Running purge command with action parameter does not purge or truncates volumes
DescriptionRunning "purge volume=VolName action storage=StorageName pool=PoolName", where pool PoolName has directive Action On Purge = Truncate does not purge or truncate the volume.

Output of the command shows "No Volumes found to perform all action.".
Steps To Reproduce1. Define pool resource with directive Action On Purge = Truncate
2. Backup some data to this pool, so we have some volumes to work with
3. From bconsole run command: purge volume=VolName action storage=StorageName pool=PoolName
4. See "No Volumes found to perform all action." on the output, Job records associated with this pool still in the Catalog and Volume file size unchanged
Additional InformationThe cause of this issue is that the director first searches for Purged volumes (before purging them) to perform the action on, finds none and bails out.

Patch attached.
TagsNo tags attached.
bareos-master: impact
bareos-master: action
bareos-19.2: impact
bareos-19.2: action
bareos-18.2: impact
bareos-18.2: action
bareos-17.2: impact
bareos-17.2: action
bareos-16.2: impact
bareos-16.2: action
bareos-15.2: impact
bareos-15.2: action
bareos-14.2: impact
bareos-14.2: action
bareos-13.2: impact
bareos-13.2: action
bareos-12.4: impact
bareos-12.4: action

Activities

jkhradil

jkhradil

2014-02-25 12:27

reporter  

0001-Fix-for-purge-command-not-purging-and-truncating-vol.patch (1,295 bytes)
From 1c04bb63f3e0aa939c9b0549daae39f66047025a Mon Sep 17 00:00:00 2001
From: Jakub Hradil <jkhradil@gmail.com>
Date: Tue, 25 Feb 2014 11:30:50 +0100
Subject: [PATCH] Fix for purge command not purging and truncating volumes when
 run with action parameter

---
 src/dird/ua_purge.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/dird/ua_purge.c b/src/dird/ua_purge.c
index 865eaca..44d5473 100644
--- a/src/dird/ua_purge.c
+++ b/src/dird/ua_purge.c
@@ -141,11 +141,6 @@ int purge_cmd(UAContext *ua, const char *cmd)
       }
    /* Volume */
    case 2:
-      /* Perform ActionOnPurge (action=truncate) */
-      if (find_arg(ua, "action") >= 0) {
-         return action_on_purge_cmd(ua, ua->cmd);
-      }
-
       while ((i=find_arg(ua, NT_("volume"))) >= 0) {
          if (select_media_dbr(ua, &mr)) {
             purge_jobs_from_volume(ua, &mr, /*force*/true);
@@ -153,6 +148,11 @@ int purge_cmd(UAContext *ua, const char *cmd)
          *ua->argk[i] = 0;            /* zap keyword already seen */
          ua->send_msg("\n");
       }
+
+      /* Perform ActionOnPurge (action=truncate) */
+      if (find_arg(ua, "action") >= 0) {
+         return action_on_purge_cmd(ua, ua->cmd);
+      }
       return 1;
    /* Quota */
    case 3:
-- 
1.8.5.3

jkhradil

jkhradil

2014-02-25 17:21

reporter   ~0000821

Fix committed to bareos bareos-12.4 branch with changesetid 1529.
jkhradil

jkhradil

2014-02-25 17:21

reporter   ~0000824

Fix committed to bareos bareos-13.2 branch with changesetid 1540.
jkhradil

jkhradil

2014-02-25 17:21

reporter   ~0000825

Fix committed to bareos master branch with changesetid 1531.
mvwieringen

mvwieringen

2014-02-27 15:19

developer   ~0000827

As it seems its was working although in a strange way. The fix above makes it trucate all volumes which is also don't what we want. So for now we reverted the
patch and will check the documentation of the feature. Eventually we may need
to add a feature request to make it somewhat better understandable or redesign
the whole feature.
pstorz

pstorz

2014-02-27 15:24

administrator   ~0000828

I looked a bit into the truncate feature and it is now implemented that you have to call purge without "action" once to purge all jobs from the database, and once again with "action" to really truncate the volume.

I agree that this is a very strange behaviour, but your patch unfortunately truncates all volumes, not only the ones specified and thus breaks the action-on-purge regression test.

It would be nice if you could implement something that works like your first fix, but does really only truncate the volume that was specified.

Thanks for helping us to improve bareos
jkhradil

jkhradil

2014-02-27 16:43

reporter   ~0000830

I kinda noticed that it truncates all purged volumes, but even with the original code, it could happen that even volumes that are not specified in the command get truncated. Besides what's the harm in truncating the volumes if they're already purged, barring someone is planning to bscan them back into the catalog.

But alright, I'm going to look into making action_on_purge_cmd function operate only on specified volumes.
jkhradil

jkhradil

2014-05-19 15:52

reporter   ~0000873

I have a code ready that truncates only the selected volume on the first run of a purge command. Before submitting a patch, I'd like to confer on one related adjustment I'd like to make.

Purge command purges all of the selected volumes (in case you run it like "purge action pool=mypool volume=volume1 volume=volume2 volume volume volume"), action_on_purge_cmd function however currently only truncates the last selected volume.

To make the behaviour consistent either the purge command needs to operate only on the last selected volume or action_on_purge_cmd function has to truncate all the selected volumes. Since I don't see an downside to allowing purging multiple volumes in one command, I suggest going with the latter solution. If it's ok, I'm going to implement necessary changes to action_on_purge_cmd function to truncate all of the selected volumes and send them in a patch together with changes I have ready for the purge_cmd function.
pstorz

pstorz

2014-05-19 16:48

administrator   ~0000874

Hello Jakub,

what you suggest sounds reasonable.
I agree that the way it is now implemented strange, and I don't see why your suggestions should cause problems, so please go on.

Thanks for your help to improve bareos!
jkhradil

jkhradil

2014-05-28 00:12

reporter  

0002-Fix-for-purge-command-to-purge-and-truncate-volumes-.patch (7,222 bytes)
From 9b6ea3a965af2695754fa47f39028e35988cfe4f Mon Sep 17 00:00:00 2001
From: Jakub Hradil <jkhradil@gmail.com>
Date: Tue, 27 May 2014 23:34:29 +0200
Subject: [PATCH] Fix for purge command to purge and truncate volumes in a
 single run

---
 src/cats/protos.h   |  2 +-
 src/cats/sql_get.c  | 25 ++++++++++++++++++-------
 src/dird/ua_purge.c | 41 ++++++++++++++++++++++++++++++++++-------
 3 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/src/cats/protos.h b/src/cats/protos.h
index fcca71c..3db8123 100644
--- a/src/cats/protos.h
+++ b/src/cats/protos.h
@@ -91,7 +91,7 @@ int db_get_num_media_records(JCR *jcr, B_DB *mdb);
 int db_get_num_pool_records(JCR *jcr, B_DB *mdb);
 int db_get_pool_ids(JCR *jcr, B_DB *mdb, int *num_ids, DBId_t **ids);
 bool db_get_client_ids(JCR *jcr, B_DB *mdb, int *num_ids, DBId_t **ids);
-bool db_get_media_ids(JCR *jcr, B_DB *mdb, MEDIA_DBR *mr, int *num_ids, uint32_t **ids);
+bool db_get_media_ids(JCR *jcr, B_DB *mdb, MEDIA_DBR *mr, POOL_MEM *volumes, int *num_ids, uint32_t **ids);
 int db_get_job_volume_parameters(JCR *jcr, B_DB *mdb, JobId_t JobId, VOL_PARAMS **VolParams);
 bool db_get_client_record(JCR *jcr, B_DB *mdb, CLIENT_DBR *cdbr);
 bool db_get_counter_record(JCR *jcr, B_DB *mdb, COUNTER_DBR *cr);
diff --git a/src/cats/sql_get.c b/src/cats/sql_get.c
index 057e5f2..b4df0d2 100644
--- a/src/cats/sql_get.c
+++ b/src/cats/sql_get.c
@@ -861,14 +861,15 @@ int db_get_num_media_records(JCR *jcr, B_DB *mdb)
 
 /**
  * This function returns a list of all the Media record ids for
- *     the current Pool, the correct Media Type, Recyle, Enabled, StorageId, VolBytes
- *     VolumeName if specified
+ *     the current Pool, the correct Media Type, Recyle, Enabled, StorageId, VolBytes and
+ *     volumes or VolumeName if specified
+ *  Comma separated list of volumes takes precedence over VolumeName.
  *  The caller must free ids if non-NULL.
  *
  *  Returns false: on failure
  *          true:  on success
  */
-bool db_get_media_ids(JCR *jcr, B_DB *mdb, MEDIA_DBR *mr, int *num_ids, uint32_t *ids[])
+bool db_get_media_ids(JCR *jcr, B_DB *mdb, MEDIA_DBR *mr, POOL_MEM *volumes, int *num_ids, uint32_t *ids[])
 {
    SQL_ROW row;
    int i = 0;
@@ -877,10 +878,15 @@ bool db_get_media_ids(JCR *jcr, B_DB *mdb, MEDIA_DBR *mr, int *num_ids, uint32_t
    bool ok = false;
    char buf[MAX_NAME_LENGTH*3]; /* Can contain MAX_NAME_LENGTH*2+1 + AND ....='' */
    char esc[MAX_NAME_LENGTH*2+1];
+   bool have_volumes = false;
 
    db_lock(mdb);
    *ids = NULL;
 
+   if (volumes != NULL) {
+      if (*volumes->c_str() != 0) have_volumes = true;
+   }
+
    Mmsg(mdb->cmd, "SELECT DISTINCT MediaId FROM Media WHERE Recycle=%d AND Enabled=%d ",
         mr->Recycle, mr->Enabled);
 
@@ -905,15 +911,20 @@ bool db_get_media_ids(JCR *jcr, B_DB *mdb, MEDIA_DBR *mr, int *num_ids, uint32_t
       pm_strcat(mdb->cmd, buf);
    }
 
-   if (*mr->VolumeName) {
+   if (*mr->VolStatus) {
+      db_escape_string(jcr, mdb, esc, mr->VolStatus, strlen(mr->VolStatus));
+      bsnprintf(buf, sizeof(buf), "AND VolStatus = '%s' ", esc);
+      pm_strcat(mdb->cmd, buf);
+   }
+
+   if (*mr->VolumeName && !have_volumes) {
       db_escape_string(jcr, mdb, esc, mr->VolumeName, strlen(mr->VolumeName));
       bsnprintf(buf, sizeof(buf), "AND VolumeName = '%s' ", esc);
       pm_strcat(mdb->cmd, buf);
    }
 
-   if (*mr->VolStatus) {
-      db_escape_string(jcr, mdb, esc, mr->VolStatus, strlen(mr->VolStatus));
-      bsnprintf(buf, sizeof(buf), "AND VolStatus = '%s' ", esc);
+   if (have_volumes) {
+      bsnprintf(buf, sizeof(buf), "AND VolumeName IN (%s) ", volumes->c_str());
       pm_strcat(mdb->cmd, buf);
    }
 
diff --git a/src/dird/ua_purge.c b/src/dird/ua_purge.c
index 9f41430..62a4e9b 100644
--- a/src/dird/ua_purge.c
+++ b/src/dird/ua_purge.c
@@ -62,6 +62,8 @@ int purge_cmd(UAContext *ua, const char *cmd)
    CLIENTRES *client;
    MEDIA_DBR mr;
    JOB_DBR  jr;
+   POOLMEM *cmd_holder = get_pool_memory(PM_FNAME);
+
    static const char *keywords[] = {
       NT_("files"),
       NT_("jobs"),
@@ -141,10 +143,8 @@ int purge_cmd(UAContext *ua, const char *cmd)
       }
    /* Volume */
    case 2:
-      /* Perform ActionOnPurge (action=truncate) */
-      if (find_arg(ua, "action") >= 0) {
-         return action_on_purge_cmd(ua, ua->cmd);
-      }
+      /* Store cmd for later reuse */
+      pm_memcpy(cmd_holder, ua->cmd, sizeof_pool_memory(ua->cmd));
 
       while ((i=find_arg(ua, NT_("volume"))) >= 0) {
          if (select_media_dbr(ua, &mr)) {
@@ -152,6 +152,24 @@ int purge_cmd(UAContext *ua, const char *cmd)
          }
          *ua->argk[i] = 0;            /* zap keyword already seen */
          ua->send_msg("\n");
+
+         /*
+          * Add volume=mr.VolumeName to cmd_holder if we have a new volume name from interactive selection.
+          * In certain cases this can produce duplicates, which we don't prevent as there are no side effects.
+          */
+         if (!bstrcmp(ua->cmd, cmd_holder)) {
+            pm_strcat(cmd_holder, " volume=");
+            pm_strcat(cmd_holder, mr.VolumeName);
+         }
+      }
+
+      /* Restore ua args based on cmd_holder */
+      pm_memcpy(ua->cmd, cmd_holder, sizeof_pool_memory(cmd_holder));
+      parse_args(ua->cmd, &ua->args, &ua->argc, ua->argk, ua->argv, MAX_CMD_ARGS);
+
+      /* Perform ActionOnPurge (action=truncate) */
+      if (find_arg(ua, "action") >= 0) {
+         return action_on_purge_cmd(ua, ua->cmd);
       }
       return 1;
    /* Quota */
@@ -191,6 +209,7 @@ int purge_cmd(UAContext *ua, const char *cmd)
          purge_quota_from_client(ua, client);
       }
    }
+   free_pool_memory(cmd_holder);
    return 1;
 }
 
@@ -695,6 +714,9 @@ static int action_on_purge_cmd(UAContext *ua, const char *cmd)
    MEDIA_DBR mr;
    POOL_DBR pr;
    BSOCK *sd = NULL;
+   char buf[MAX_NAME_LENGTH*3]; /* Can contain MAX_NAME_LENGTH*2+1 + AND ....='' */
+   char esc[MAX_NAME_LENGTH*2+1];
+   POOL_MEM volumes(PM_FNAME);
 
    memset(&pr, 0, sizeof(pr));
 
@@ -705,8 +727,13 @@ static int action_on_purge_cmd(UAContext *ua, const char *cmd)
 
       } else if (bstrcasecmp(ua->argk[i], NT_("volume")) &&
                  is_name_valid(ua->argv[i], NULL)) {
-         bstrncpy(mr.VolumeName, ua->argv[i], sizeof(mr.VolumeName));
-
+         db_escape_string(ua->jcr, ua->db, esc, ua->argv[i], strlen(ua->argv[i]));
+         if (*volumes.c_str() == 0) {
+            bsnprintf(buf, sizeof(buf), "'%s'", esc);
+         } else {
+            bsnprintf(buf, sizeof(buf), ",'%s'", esc);
+         }
+         pm_strcat(volumes, buf);
       } else if (bstrcasecmp(ua->argk[i], NT_("devicetype")) &&
                  ua->argv[i]) {
          bstrncpy(mr.MediaType, ua->argv[i], sizeof(mr.MediaType));
@@ -765,7 +792,7 @@ static int action_on_purge_cmd(UAContext *ua, const char *cmd)
    mr.VolBytes = 10000;
    set_storageid_in_mr(store, &mr);
    bstrncpy(mr.VolStatus, "Purged", sizeof(mr.VolStatus));
-   if (!db_get_media_ids(ua->jcr, ua->db, &mr, &nb, &results)) {
+   if (!db_get_media_ids(ua->jcr, ua->db, &mr, &volumes, &nb, &results)) {
       Dmsg0(100, "No results from db_get_media_ids\n");
       goto bail_out;
    }
-- 
1.8.5.3

jkhradil

jkhradil

2014-05-28 00:47

reporter   ~0000885

New version of a patch is attached to the ticket.

Purge command now first purges selected volumes and then truncates them.

Changes to purge_cmd put purging and truncating in the correct order and deal with volume keywords being zapped and thus not being availeble to action_on_purge_cmd function.

The rest of the changes allows to get accurate list of candidate volumes for purging.
mvwieringen

mvwieringen

2014-05-31 10:27

developer   ~0000888

Ok thanks for the patch we will evaluate it and do some tests with it and then
come back to you with either some questions or by integrating it into the
master branch. We might do some tweaking of the code to make it somewhat more
uniform to the other code but from a first scan things look OK.

As we don't have a lot of experience with this particular part of the code
(probably nobody has as it had little usability before this patch) it could
take a couple of days before we have any progress.
ogagnon

ogagnon

2015-02-21 13:54

reporter   ~0001279

In addition to these issue, I suggest to add the following feature.

I have looked at the code and the "action=truncate" parameter is redundent since in file "dird/ua_purge.c" line 177:

/*
* Perform ActionOnPurge (action=truncate)
*/
if (find_arg(ua, "action") >= 0) {
    return action_on_purge_cmd(ua, ua->cmd);
}

In "include/baconfig.h", you have at line 224:

/**
* Actions on purge (bit mask)
*/
#define ON_PURGE_NONE 0
#define ON_PURGE_TRUNCATE 1

-----

When working with File volumes that are configured to be used once, truncating the volume on purge is not enough. Another action called "delete" or "remove" should be added to remove the File volume from the filesystem and the Catalog.

Scenario:
I have a Pool for Differential backup on a weekly basis. Volumes are auto labeled "Weekly-$day-$month-$year". I configure the Pool to have 4 Volumes max with a retention period of 21 days. When a fifth volume is going to be created, the oldest Volume in the pool has expired and is then Purged leaving an open slot to create the new Volume.

However, that oldest Volume is still on the filesystem and in the Catalog. I have to remove it manually.

What I am doing now is that I created a Pool named "ToDelete" and configured the Weekly Pool with the parameter "Recycle Pool = ToDelete". So, when the oldest Volume is purged, it is moved to the Pool "ToDelete", leaving a free slot to create another one. But, I still have to look at the Volumes in that Pool every now and then to really remove them. Since I have using more than one SD, I also have to get into the right SD to do the removal.

So, that's my suggestion. I've looked to do that with a plugin but the Director Event "bDirEventVolumePurged" does not give enough information.

O.
mvwieringen

mvwieringen

2015-03-25 16:51

developer   ~0001352

Fix committed to bareos2015 bareos-14.2 branch with changesetid 4898.

Related Changesets

bareos: bareos-12.4 e30d3b90

2014-02-25 10:30:50

jkhradil

Ported: N/A

Details Diff
Fix for purge command not purging and truncating.

Fix for purge command not purging and truncating volumes when run
with action parameter

Fixes 0000282: Running purge command with action parameter does not purge
or truncates volumes

Signed-off-by: Marco van Wieringen <marco.van.wieringen@bareos.com>
Affected Issues
0000282
mod - src/dird/ua_purge.c Diff File

bareos: master dc89f661

2014-02-25 10:30:50

jkhradil

Ported: N/A

Details Diff
Fix for purge command not purging and truncating.

Fix for purge command not purging and truncating volumes when run
with action parameter

Fixes 0000282: Running purge command with action parameter does not purge
or truncates volumes

Signed-off-by: Marco van Wieringen <marco.van.wieringen@bareos.com>
Affected Issues
0000282
mod - src/dird/ua_purge.c Diff File

bareos: bareos-13.2 6c8d624d

2014-02-25 10:30:50

jkhradil

Ported: N/A

Details Diff
Fix for purge command not purging and truncating.

Fix for purge command not purging and truncating volumes when run
with action parameter

Fixes 0000282: Running purge command with action parameter does not purge
or truncates volumes

Signed-off-by: Marco van Wieringen <marco.van.wieringen@bareos.com>
Affected Issues
0000282
mod - src/dird/ua_purge.c Diff File

bareos2015: bareos-12.4 4c4870b0

2014-02-25 11:30:50

jkhradil


Committer: mvwieringen

Ported: N/A

Details Diff
Fix for purge command not purging and truncating.

Fix for purge command not purging and truncating volumes when run
with action parameter

Fixes 0000282: Running purge command with action parameter does not purge
or truncates volumes

Signed-off-by: Marco van Wieringen <marco.van.wieringen@bareos.com>
Affected Issues
0000282
mod - src/dird/ua_purge.c Diff File

bareos2015: bareos-13.2 a8c9516e

2014-02-25 11:30:50

jkhradil


Committer: mvwieringen

Ported: N/A

Details Diff
Fix for purge command not purging and truncating.

Fix for purge command not purging and truncating volumes when run
with action parameter

Fixes 0000282: Running purge command with action parameter does not purge
or truncates volumes

Signed-off-by: Marco van Wieringen <marco.van.wieringen@bareos.com>
Affected Issues
0000282
mod - src/dird/ua_purge.c Diff File

bareos2015: bareos-14.2 53a132bc

2014-02-25 11:30:50

jkhradil


Committer: mvwieringen

Ported: N/A

Details Diff
Fix for purge command not purging and truncating.

Fix for purge command not purging and truncating volumes when run
with action parameter

Fixes 0000282: Running purge command with action parameter does not purge
or truncates volumes

Signed-off-by: Marco van Wieringen <marco.van.wieringen@bareos.com>
Affected Issues
0000282
mod - src/dird/ua_purge.c Diff File

bareos: master 9600ee50

2014-05-27 21:34:29

jkhradil

Ported: N/A

Details Diff
Fix for purge command to purge and truncate volumes in a single run Affected Issues
0000282
mod - src/cats/protos.h Diff File
mod - src/cats/sql_get.c Diff File
mod - src/dird/ua_purge.c Diff File

Issue History

Date Modified Username Field Change
2014-02-25 12:27 jkhradil New Issue
2014-02-25 12:27 jkhradil File Added: 0001-Fix-for-purge-command-not-purging-and-truncating-vol.patch
2014-02-25 17:21 jkhradil Changeset attached => bareos bareos-12.4 e30d3b90
2014-02-25 17:21 jkhradil Note Added: 0000821
2014-02-25 17:21 jkhradil Assigned To => jkhradil
2014-02-25 17:21 jkhradil Status new => resolved
2014-02-25 17:21 jkhradil Resolution open => fixed
2014-02-25 17:21 jkhradil Changeset attached => bareos bareos-13.2 6c8d624d
2014-02-25 17:21 jkhradil Note Added: 0000824
2014-02-25 17:21 jkhradil Changeset attached => bareos master dc89f661
2014-02-25 17:21 jkhradil Note Added: 0000825
2014-02-27 15:17 mvwieringen Assigned To jkhradil => mvwieringen
2014-02-27 15:17 mvwieringen Status resolved => feedback
2014-02-27 15:17 mvwieringen Resolution fixed => reopened
2014-02-27 15:18 mvwieringen Status feedback => assigned
2014-02-27 15:19 mvwieringen Note Added: 0000827
2014-02-27 15:24 pstorz Note Added: 0000828
2014-02-27 16:43 jkhradil Note Added: 0000830
2014-02-28 11:06 mvwieringen Assigned To mvwieringen => pstorz
2014-05-19 15:52 jkhradil Note Added: 0000873
2014-05-19 16:48 pstorz Note Added: 0000874
2014-05-28 00:12 jkhradil File Added: 0002-Fix-for-purge-command-to-purge-and-truncate-volumes-.patch
2014-05-28 00:47 jkhradil Note Added: 0000885
2014-05-31 10:27 mvwieringen Note Added: 0000888
2014-06-24 14:42 mvwieringen Changeset attached => bareos master 9600ee50
2015-02-21 13:54 ogagnon Note Added: 0001279
2015-03-25 16:51 mvwieringen Changeset attached => bareos2015 bareos-12.4 4c4870b0
2015-03-25 16:51 mvwieringen Changeset attached => bareos2015 bareos-13.2 a8c9516e
2015-03-25 16:51 mvwieringen Changeset attached => bareos2015 bareos-14.2 53a132bc
2015-03-25 16:51 mvwieringen Note Added: 0001352
2015-03-25 16:51 mvwieringen Status assigned => resolved
2015-03-31 15:25 mvwieringen adm Assigned To pstorz =>