EnterpriseDB / barman

Barman - Backup and Recovery Manager for PostgreSQL
https://www.pgbarman.org/
GNU General Public License v3.0
2.06k stars 191 forks source link

Consume queue during cron taking care of config-update interruptions #886

Closed gcalacoci closed 8 months ago

edb-sonar-app[bot] commented 8 months ago

Quality Gate passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
0.0% 0.0% Coverage on New Code
0.0% 0.0% Duplication on New Code

See analysis details on SonarQube

barthisrael commented 8 months ago

I was wondering about something else. The operation should be quite quick. However, it will grab the lock each time barman cron runs, even if the file is empty. What if someone or something calls barman config-update at the same tiem barman cron is already checking the empty queue?

gcalacoci commented 8 months ago

Looks good to me. Should we add unit tests to cover this?

UT for ConfigChangesProcessor are part of the bar-128 pr we merged. in general cli.py has no UT

gcalacoci commented 8 months ago

I was wondering about something else. The operation should be quite quick. However, it will grab the lock each time barman cron runs, even if the file is empty. What if someone or something calls barman config-update at the same tiem barman cron is already checking the empty queue?

well. the config update will fail and tbh I'm fine with that. we don't want to have multiple processes messing with config files at the same time. as the corn will potentially modify them IMHO is worth the risk of a conflict.

barthisrael commented 8 months ago

Ok with both points, makes sense. Let's go for it and merge this PR.