View Issue Details

IDProjectCategoryView StatusLast Update
0001227bareos-core[All Projects] file daemonpublic2020-06-09 15:15
ReportergjeluAssigned Toarogge 
PrioritynormalSeverityminorReproducibilityN/A
Status resolvedResolutionfixed 
PlatformLinuxOSRHELOS Version7
Product Version19.2.6 
Fixed in Version 
Summary0001227: Fix fadvise bug - sync bacula commit
DescriptionDuring backups we notice on our RHEL systems that the complete memory gets occupied by Page Cache.

An explanation of the cause can already by found in this mail thread: http://bacula.10910.n7.nabble.com/Support-for-HAVE-POSIX-FADVISE-on-Bacula-Client-td82345.html

In bacula this has been fixed using following commit "Fix fadvise bug found by Robert Heinzmann":
https://www.bacula.org/git/cgit.cgi/bacula/commit/bacula/src/findlib/bfile.c?id=e5039ea45e6ab752c9c57c072c8a58f52a31e603

Would be great if this fix can also applied in bareos.
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

arogge

arogge

2020-04-17 15:03

developer   ~0003942

What is the use-case you need this for? The whole point of a page cache is to do exactly what it does there.
However, the current code is apparently broken, as the check for the filemode is simply implemented wrong.
Fixing this would introduce a call to fdatasync() for every file read by the filedaemon. I don't think this is a good idea at all. Either the page cache is clean (and can be released without fdatasync()) or the page cache is unclean and it is definitely not our duty to fdatasync() files we have just been reading.
gjelu

gjelu

2020-04-23 18:00

reporter   ~0003963

From the bacula mail thread I can understand that they did make the same observations but nevertheless decided that implementing it was an overall better solution: see
http://bacula.10910.n7.nabble.com/Support-for-HAVE-POSIX-FADVISE-on-Bacula-Client-tp82345p82361.html

What we notice is that RAM memory is completely in use by the page cache.
As the files are not evicted from the page cache, they consume RAM after the files were being read by the bareos-fd process.
Some processes (on servers with memory intensive processes) are getting swap space as the kernel might think this is more beneficial opposed to evicting entries from the page cache.
Mostly this is around the time of the backup process.
So I am not arguing against the page cache. Because the “fix” will still use the page cache, but with FADV_DONTNEED it will immediately be released from the page cache. And that is why it's important to us.
arogge

arogge

2020-04-24 10:15

developer   ~0003964

The problem I see is that the FADV_DONTNEED will also tell the kernel to release data from the page cache that may be in use by other processes. I hope modern kernels are smart enough not to release the cached data in these cases.
if you want the fix applied, please prepare a pull request on github with the change so I can discuss it with the other core developers. I think there is a good chance we will accept this.
gjelu

gjelu

2020-04-27 14:07

reporter   ~0003965

As per request, I've created https://github.com/bareos/bareos/pull/496
gjelu

gjelu

2020-05-11 17:24

reporter   ~0003987

Fix committed to bareos master branch with changesetid 13301.

Related Changesets

bareos: master ea666394

2020-04-30 14:50:33

gjelu

Ported: N/A

Details Diff
Do not call fdatasync() and fix m_flags->flags_

Fixes 0001227: Fix fadvise bug

Suggested changes implemented:
- https://github.com/bareos/bareos/pull/496#discussion_r417897291
- https://github.com/bareos/bareos/pull/496#discussion_r417889911
Affected Issues
0001227
mod - core/src/findlib/bfile.cc Diff File

Issue History

Date Modified Username Field Change
2020-04-10 13:57 gjelu New Issue
2020-04-17 15:03 arogge Assigned To => arogge
2020-04-17 15:03 arogge Status new => feedback
2020-04-17 15:03 arogge Note Added: 0003942
2020-04-23 18:00 gjelu Note Added: 0003963
2020-04-23 18:00 gjelu Status feedback => assigned
2020-04-24 10:15 arogge Status assigned => feedback
2020-04-24 10:15 arogge Note Added: 0003964
2020-04-27 14:07 gjelu Note Added: 0003965
2020-04-27 14:07 gjelu Status feedback => assigned
2020-05-11 17:24 gjelu Changeset attached => bareos master ea666394
2020-05-11 17:24 gjelu Note Added: 0003987
2020-06-09 15:15 arogge Status assigned => resolved
2020-06-09 15:15 arogge Resolution open => fixed