consolidate: purge jobids without files before starting the consolidation #1056

pstorz · 2022-01-25T12:05:47Z

Thank you for contributing to the Bareos Project!

The consolidation job now purges jobs without files from the jobid list before starting the consolidation. This makes sure that jobs without are cleaned up before and consolidation of jobs that do not have any files is never tried.

Also, the current postgresql query code reacts to an PGRES_FATAL_ERROR either by exiting the daemon (if exit on fatal is set) or by simply returning a normal error returning the error string from the database.

Please check

Short description and the purpose of this PR is present above this paragraph
Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General

PR name is meaningful
Purpose of the PR is understood
Separate commit for this PR in the CHANGELOG.md, PR number referenced is same
Commit descriptions are understandable and well formatted

Source code quality

Source code changes are understandable
Variable and function names are meaningful
Code comments are correct (logically and spelling)
Required documentation changes are present and part of the PR
bareos-check-sources --since-merge does not report any problems
git status should not report modifications in the source tree after building and testing

Tests

Decision taken that a system- or unittest is required (if not, then remove this paragraph)
The decision towards a systemtest is reasonable compared to a unittest
Testname matches exactly what is being tested
Output of the test leads quickly to the origin of the fault

core/src/cats/postgresql.cc

bruno-at-bareos

Answer the question about sending the code in case of exit on fatal.
Changelog Commit needed.

bruno-at-bareos

I let one suggestion open.

core/src/console/console.cc

bruno-at-bareos

Let see if last modification build and tests pass.

bruno-at-bareos

Everything reviewed, tested, and build correctly.
Nice work.

arogge · 2022-02-21T10:33:29Z

core/src/cats/sql_get.cc

+{
+  db_list_ctx deleted_jobids;
+  std::string query{
+      "DELETE FROM Job where (JobFiles=0 OR PurgedFiles=1) AND JobID in ("};


do we really want to implicitly purge jobs when their files were purged?
I don't think that's right.

also, just doing an SQL DELETE will potentially leave stale records in tables JobMedia, Log, basefiles, PathVisibility, NDMPJobEnvironment, JobStats and RestoreObject.

I guess we need to determine the list and then do whatever"delete jobid=XXX" would be doing.

arogge · 2022-02-21T10:38:10Z

core/src/cats/sql_get.cc

+{
+  db_list_ctx deleted_jobids;
+  std::string query{
+      "DELETE FROM Job where (JobFiles=0 OR PurgedFiles=1) AND JobID in ("};


also, just doing an SQL DELETE will potentially leave stale records in tables JobMedia, Log, basefiles, PathVisibility, NDMPJobEnvironment, JobStats and RestoreObject.

arogge · 2022-02-21T10:42:01Z

core/src/cats/sql_get.cc

+  if (!SqlQueryWithHandler(query.c_str(), DbDeleteListHandler,
+                           &deleted_jobids)) {
+    Jmsg(jcr, M_ERROR, 0,
+         "Deleting jobids with no files from list %s failed : ERR=%s\n",


shouldn't it be "Purging" instead of "Deleting"?

arogge · 2022-02-21T10:45:09Z

core/src/cats/sql_get.cc

+{
+  db_list_ctx deleted_jobids;
+  std::string query{
+      "DELETE FROM Job where (JobFiles=0 OR PurgedFiles=1) AND JobID in ("};


I guess we need to determine the list and then do whatever"delete jobid=XXX" would be doing.

in case of PGRES_FATAL_ERROR

Verify that empty jobs are purged before consolidation starts.

This patch refactors functions to purge files and jobs and to upgrade copy jobs to backup jobs so they can be called without an UaContext.

Previously empty jobs (i.e. those with JobFiles == 0) were left untouched by consolidation. This change now adds functionality that will identify these jobs and purge them so the result looks like they had been consolidated, too.

Done

bruno-at-bareos self-assigned this Jan 26, 2022

bruno-at-bareos reviewed Jan 26, 2022

View reviewed changes

core/src/cats/postgresql.cc Outdated Show resolved Hide resolved

bruno-at-bareos reviewed Jan 26, 2022

View reviewed changes

pstorz added the nobuild label Jan 26, 2022

bruno-at-bareos reviewed Jan 26, 2022

View reviewed changes

core/src/console/console.cc Outdated Show resolved Hide resolved

pstorz force-pushed the dev/pstorz/master/sql-fatal-error-logging branch from 76b2a6c to f9592a7 Compare February 9, 2022 14:42

pstorz removed the nobuild label Feb 10, 2022

pstorz changed the title ~~postgresql: print PQerrorMessage() in case of PGRES_FATAL_ERROR~~ consolidate: purge jobids without files before starting the consolidation Feb 11, 2022

pstorz force-pushed the dev/pstorz/master/sql-fatal-error-logging branch from 8098589 to 4c3dc77 Compare February 14, 2022 13:30

bruno-at-bareos force-pushed the dev/pstorz/master/sql-fatal-error-logging branch from 4c17c41 to 5680781 Compare February 16, 2022 16:55

bruno-at-bareos reviewed Feb 16, 2022

View reviewed changes

bruno-at-bareos approved these changes Feb 17, 2022

View reviewed changes

bruno-at-bareos added the requires backport to 21 label Feb 17, 2022

arogge reviewed Feb 21, 2022

View reviewed changes

arogge previously requested changes Feb 21, 2022

View reviewed changes

arogge self-assigned this Feb 24, 2022

arogge force-pushed the dev/pstorz/master/sql-fatal-error-logging branch 2 times, most recently from e044bb8 to 3327e62 Compare March 7, 2022 14:48

pstorz and others added 4 commits March 7, 2022 15:57

postgresql: print PQerrorMessage()

1888f44

in case of PGRES_FATAL_ERROR

systemtests: ai-consolidate-test purge empty jobs

0198b24

Verify that empty jobs are purged before consolidation starts.

cats: refactor functionality from UA to DB

46cb49e

This patch refactors functions to purge files and jobs and to upgrade copy jobs to backup jobs so they can be called without an UaContext.

dird: purge empty jobs as part of consolidation

c6306fa

Previously empty jobs (i.e. those with JobFiles == 0) were left untouched by consolidation. This change now adds functionality that will identify these jobs and purge them so the result looks like they had been consolidated, too.

arogge force-pushed the dev/pstorz/master/sql-fatal-error-logging branch from 3327e62 to c6306fa Compare March 7, 2022 14:59

Update CHANGELOG.md

538697d

arogge force-pushed the dev/pstorz/master/sql-fatal-error-logging branch from 4a167a4 to 538697d Compare March 7, 2022 17:13

pstorz mentioned this pull request Mar 8, 2022

consolidate: purge jobids without files before starting the consolidation #1107

Merged

17 tasks

pstorz merged commit 9472579 into master Mar 9, 2022

arogge deleted the dev/pstorz/master/sql-fatal-error-logging branch March 9, 2022 16:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

consolidate: purge jobids without files before starting the consolidation #1056

consolidate: purge jobids without files before starting the consolidation #1056

pstorz commented Jan 25, 2022 •

edited by bruno-at-bareos

Loading

bruno-at-bareos left a comment

bruno-at-bareos left a comment

bruno-at-bareos left a comment

bruno-at-bareos left a comment

arogge Feb 21, 2022

arogge Feb 21, 2022

arogge Feb 21, 2022

arogge Feb 21, 2022

arogge Feb 21, 2022

arogge Feb 21, 2022

consolidate: purge jobids without files before starting the consolidation #1056

consolidate: purge jobids without files before starting the consolidation #1056

Conversation

pstorz commented Jan 25, 2022 • edited by bruno-at-bareos Loading

Thank you for contributing to the Bareos Project!

Please check

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General

Source code quality

Tests

bruno-at-bareos left a comment

Choose a reason for hiding this comment

bruno-at-bareos left a comment

Choose a reason for hiding this comment

bruno-at-bareos left a comment

Choose a reason for hiding this comment

bruno-at-bareos left a comment

Choose a reason for hiding this comment

arogge Feb 21, 2022

Choose a reason for hiding this comment

arogge Feb 21, 2022

Choose a reason for hiding this comment

arogge Feb 21, 2022

Choose a reason for hiding this comment

arogge Feb 21, 2022

Choose a reason for hiding this comment

arogge Feb 21, 2022

Choose a reason for hiding this comment

arogge Feb 21, 2022

Choose a reason for hiding this comment

pstorz commented Jan 25, 2022 •

edited by bruno-at-bareos

Loading