From: Robert H. <r.h...@fr...> - 2015-10-07 09:35:53
|
Hello, we are using bacula on a virtual plattform (> 600 clients) and investiate moderate swap usage issues / cache usage issues. It seems that during backup of mysql datafiles (LVM Snapshot), the "cached" memory usage of the server increases and thus bacula backup reads (only done one) are cached (... polluting the rest of the vfs cache ?) >From reading the source of src/findlib/bfile.c, bacula supports posix_fadvise for a long time now (2007), by those two code pices: commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31 Author: Kern Sibbald <ke...@si...> Date: Thu May 24 19:58:07 2007 +0000 kes Add code to tell the OS that we no longer need a cached file that we were reading. In findlib/bfile.c. Also, only cache files that we are reading. kes Tweak to bsmtp to eliminate compiler warnings on Win32. kes Implement script to automatically generate cats and dll .def files for Win32 dll. kes Update README.mingw32 to include new .def file generation. git-svn-id: https://bacula.svn.sourceforge.net/svnroot/bacula/trunk@4898 91ce42f0-d328-0410-95d8-f526ca767f89 bopen() #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED) if (bfd->fid != -1 && flags & O_RDONLY) { int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED); Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat); } #endif bclose() #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED) if (bfd->m_flags & O_RDONLY) { fdatasync(bfd->fid); /* sync the file */ /* Tell OS we don't need it any more */ posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED); } #endif I checked that our bacula client was compiled with HAVE_POSIX_FADVISE (added debug messages). I also added more debugging code and it seems that during a normal backup run, the code is never executed on bacula-fd as O_RDONLY is not set in the flags of the originating bopen request. Is this a bug or a feature ? How can I prevent cache usage of bacula-fd on the client side at all ? Is there a Flag in bacula-fd to force O_READONLY opening of backed up files ? Regards, Robert |
From: Robert H. <r.h...@fr...> - 2015-10-07 11:57:41
|
Hello, just a short followup. It seems I found the issue causing our file system cache to fill up during backup jobs and our virtual platform memory usage to increase. The O_RDONLY mask is "00" on Linux, casing the if flags & O_RDONLY to always fail. So the patch regarding posix_fadvise() in commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31 is actually dead code. (if (Anything & 00) { dead code; }) This diff fixes the issue: diff --git a/bacula/src/findlib/bfile.c b/bacula/src/findlib/bfile.c index e315675..29f04a3 100644 --- a/bacula/src/findlib/bfile.c +++ b/bacula/src/findlib/bfile.c @@ -997,9 +997,9 @@ int bopen(BFILE *bfd, const char *fname, int flags, mode_t mode) bfd->win32DecompContext.liNextHeader = 0; #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED) - if (bfd->fid != -1 && flags & O_RDONLY) { + if (bfd->fid != -1 && flags == O_RDONLY) { int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED); - Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat); + Dmsg2(400, "Did posix_fadvise POSIX_FADV_WILLNEED on %s stat=%d\n", fname, stat); } #endif @@ -1043,10 +1043,11 @@ int bclose(BFILE *bfd) return 0; } #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED) - if (bfd->m_flags & O_RDONLY) { + if (bfd->m_flags == O_RDONLY) { fdatasync(bfd->fid); /* sync the file */ /* Tell OS we don't need it any more */ posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED); + Dmsg1(400, "Did posix_fadvise POSIX_FADV_DONTNEED on File Desriptor %d\n", bfd->fid); } #endif After applying this patch, the cache memory does not increase a lot during backups on my test machine. Running widh bacula-fd -d 400 also shows the debugging messages. However this fix has a HUGE impact on behavior of existing backup jobs - altought it has huge benefits on virtual platforms with short backup windows also. Current behavior: - backup jobs with snapshots: caching of unneeded files in memory, filesystem cache is polluted, virtual platform memory usage increases - backup Jobs without snapshots: backup job warms up the cache of all files Behaviour with fix: - backup jobs with snapshots: only caching of 1 unneeded file in memory, filesystem cache is less polluted (largest file to backup), virtual platform memory usage decreases - backup Jobs without snapshots: backup job does not warms up the cache of all files, but also drops existing cache for cached files (may have performance impact for webservers, fileservers etc.) Conclusion: - As the impact on behavior is huge, fixing the bug in place can be problematic - Unfortunately the right way to impelement posix_fadvise() with POSIX_FADV_NOREUSE is not implemented on Linux (See http://lxr.free-electrons.com/source/mm/fadvise.c#L115), so there is "right" way to fix this issue - Best approach: there should be a Bacula Option to enable the POSIX_FADV_NOREUSE explicitly for selective bacula Jobs to allow usage of posix_fadvise() for direct I/O workloads (like mysql databases etc with snapshots etc.) and avoid it for webserver and fileserver workloads. Admins can decide. The default should be the current behavior to not call posix_fadvise(). Also the code path for SD/Dir und FD should be separate so enable this for the FD bfile.c code only (it seems currently it is the same code). Regards, Robert Robert Heinzmann r.h...@fr... IT-Operations Travian Games GmbH Von: Robert Heinzmann [mailto:r.h...@fr...] Gesendet: Mittwoch, 7. Oktober 2015 11:16 An: bac...@li... Betreff: [Bacula-users] Support for HAVE_POSIX_FADVISE on Bacula Client Hello, we are using bacula on a virtual plattform (> 600 clients) and investiate moderate swap usage issues / cache usage issues. It seems that during backup of mysql datafiles (LVM Snapshot), the "cached" memory usage of the server increases and thus bacula backup reads (only done one) are cached (... polluting the rest of the vfs cache ?) >From reading the source of src/findlib/bfile.c, bacula supports posix_fadvise for a long time now (2007), by those two code pices: commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31 Author: Kern Sibbald <ke...@si...<mailto:ke...@si...>> Date: Thu May 24 19:58:07 2007 +0000 kes Add code to tell the OS that we no longer need a cached file that we were reading. In findlib/bfile.c. Also, only cache files that we are reading. kes Tweak to bsmtp to eliminate compiler warnings on Win32. kes Implement script to automatically generate cats and dll .def files for Win32 dll. kes Update README.mingw32 to include new .def file generation. git-svn-id: https://bacula.svn.sourceforge.net/svnroot/bacula/trunk@4898 91ce42f0-d328-0410-95d8-f526ca767f89 bopen() #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED) if (bfd->fid != -1 && flags & O_RDONLY) { int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED); Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat); } #endif bclose() #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED) if (bfd->m_flags & O_RDONLY) { fdatasync(bfd->fid); /* sync the file */ /* Tell OS we don't need it any more */ posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED); } #endif I checked that our bacula client was compiled with HAVE_POSIX_FADVISE (added debug messages). I also added more debugging code and it seems that during a normal backup run, the code is never executed on bacula-fd as O_RDONLY is not set in the flags of the originating bopen request. Is this a bug or a feature ? How can I prevent cache usage of bacula-fd on the client side at all ? Is there a Flag in bacula-fd to force O_READONLY opening of backed up files ? Regards, Robert |
From: Gary R. S. <gr...@mc...> - 2015-10-07 13:10:13
|
On 7/10/2015 10:57 PM, Robert Heinzmann wrote: > Hello, > > just a short followup. It seems I found the issue causing our file > system cache to fill up during backup jobs and our virtual platform > memory usage to increase. > > The O_RDONLY mask is “00” on Linux, casing the if flags & O_RDONLY to > always fail. So the patch regarding posix_fadvise() in commit > 4df486dd5b04cc16178efcb75eaf3dbd4be44b31 is actually dead code. (if > (Anything & 00) { dead code; }) > It's a real bug - O_RDONLY is 0 on Solaris, AIX, HP-UX, and Windows as well. Cheers, Gary B-) |
From: Eric B. <eri...@ba...> - 2015-10-07 21:57:52
|
Hello Robert, I think you did a really nice investigation work :-) I think that your patch is almost right, and we need to figure if we want to add specific directives for that. I tend to believe right now that with or without the POSIX call, the os will cache the file, and it sounds better to do it in advance, let see what other people are thinking about your idea. Thanks, Eric Le 2015-10-07 13:57, Robert Heinzmann <r.h...@fr...> a écrit : > > Hello, > > > > just a short followup. It seems I found the issue causing our file system cache to fill up during backup jobs and our virtual platform memory usage to increase. > > > > The O_RDONLY mask is “00” on Linux, casing the if flags & O_RDONLY to always fail. So the patch regarding posix_fadvise() in commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31 is actually dead code. (if (Anything & 00) { dead code; }) > > > > This diff fixes the issue: > > > > diff --git a/bacula/src/findlib/bfile.c b/bacula/src/findlib/bfile.c > > index e315675..29f04a3 100644 > > --- a/bacula/src/findlib/bfile.c > > +++ b/bacula/src/findlib/bfile.c > > @@ -997,9 +997,9 @@ int bopen(BFILE *bfd, const char *fname, int flags, mode_t mode) > > bfd->win32DecompContext.liNextHeader = 0; > > > > #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED) > > - if (bfd->fid != -1 && flags & O_RDONLY) { > > + if (bfd->fid != -1 && flags == O_RDONLY) { > > int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED); > > - Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat); > > + Dmsg2(400, "Did posix_fadvise POSIX_FADV_WILLNEED on %s stat=%d\n", fname, stat); > > } > > #endif > > > > @@ -1043,10 +1043,11 @@ int bclose(BFILE *bfd) > > return 0; > > } > > #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED) > > - if (bfd->m_flags & O_RDONLY) { > > + if (bfd->m_flags == O_RDONLY) { > > fdatasync(bfd->fid); /* sync the file */ > > /* Tell OS we don't need it any more */ > > posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED); > > + Dmsg1(400, "Did posix_fadvise POSIX_FADV_DONTNEED on File Desriptor %d\n", bfd->fid); > > } > > #endif > > > > After applying this patch, the cache memory does not increase a lot during backups on my test machine. Running widh bacula-fd –d 400 also shows the debugging messages. > > > > However this fix has a HUGE impact on behavior of existing backup jobs - altought it has huge benefits on virtual platforms with short backup windows also. > > > > Current behavior: > > - backup jobs with snapshots: caching of unneeded files in memory, filesystem cache is polluted, virtual platform memory usage increases > > - backup Jobs without snapshots: backup job warms up the cache of all files > > > > Behaviour with fix: > > - backup jobs with snapshots: only caching of 1 unneeded file in memory, filesystem cache is less polluted (largest file to backup), virtual platform memory usage decreases > > - backup Jobs without snapshots: backup job does not warms up the cache of all files, but also drops existing cache for cached files (may have performance impact for webservers, fileservers etc.) > > > > > > Conclusion: > > - As the impact on behavior is huge, fixing the bug in place can be problematic > > - Unfortunately the right way to impelement posix_fadvise() with POSIX_FADV_NOREUSE is not implemented on Linux (See http://lxr.free-electrons.com/source/mm/fadvise.c#L115), so there is “right” way to fix this issue > > - Best approach: there should be a Bacula Option to enable the POSIX_FADV_NOREUSE explicitly for selective bacula Jobs to allow usage of posix_fadvise() for direct I/O workloads (like mysql databases etc with snapshots etc.) and avoid it for webserver and fileserver workloads. Admins can decide. The default should be the current behavior to not call posix_fadvise(). Also the code path for SD/Dir und FD should be separate so enable this for the FD bfile.c code only (it seems currently it is the same code). > > > > Regards, > > Robert > > > > Robert Heinzmann > > r.h...@fr... > > IT-Operations > > Travian Games GmbH > > > > Von: Robert Heinzmann [mailto:r.h...@fr...] > Gesendet: Mittwoch, 7. Oktober 2015 11:16 > An: bac...@li... > Betreff: [Bacula-users] Support for HAVE_POSIX_FADVISE on Bacula Client > > > > Hello, > > > > we are using bacula on a virtual plattform (> 600 clients) and investiate moderate swap usage issues / cache usage issues. It seems that during backup of mysql datafiles (LVM Snapshot), the “cached” memory usage of the server increases and thus bacula backup reads (only done one) are cached (… polluting the rest of the vfs cache ?) > > > > From reading the source of src/findlib/bfile.c, bacula supports posix_fadvise for a long time now (2007), by those two code pices: > > > > commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31 > > Author: Kern Sibbald <ke...@si...> > > Date: Thu May 24 19:58:07 2007 +0000 > > > > kes Add code to tell the OS that we no longer need a cached > > file that we were reading. In findlib/bfile.c. Also, > > only cache files that we are reading. > > kes Tweak to bsmtp to eliminate compiler warnings on Win32. > > kes Implement script to automatically generate cats and dll .def > > files for Win32 dll. > > kes Update README.mingw32 to include new .def file generation. > > > > git-svn-id: https://bacula.svn.sourceforge.net/svnroot/bacula/trunk@4898 91ce42f0-d328-0410-95d8-f526ca767f89 > > > > bopen() > > > > #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED) > > if (bfd->fid != -1 && flags & O_RDONLY) { > > int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED); > > Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat); > > } > > #endif > > > > bclose() > > > > #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED) > > if (bfd->m_flags & O_RDONLY) { > > fdatasync(bfd->fid); /* sync the file */ > > /* Tell OS we don't need it any more */ > > posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED); > > } > > #endif > > > > I checked that our bacula client was compiled with HAVE_POSIX_FADVISE (added debug messages). > > > > I also added more debugging code and it seems that during a normal backup run, the code is never executed on bacula-fd as O_RDONLY is not set in the flags of the originating bopen request. > > > > Is this a bug or a feature ? > > > > How can I prevent cache usage of bacula-fd on the client side at all ? Is there a Flag in bacula-fd to force O_READONLY opening of backed up files ? > > > > Regards, > > Robert |
From: Josh F. <jf...@pv...> - 2015-10-08 11:49:51
|
On 10/7/2015 10:49 AM, Eric Bollengier wrote: > Hello Robert, > > I think you did a really nice investigation work :-) I think that your patch is almost right, and we need to figure if we want to add specific directives for that. I tend to believe right now that with or without the POSIX call, the os will cache the file, and it sounds better to do it in advance, let see what other people are thinking about your idea. Definitely a good job in finding the bug. I still believe that it is best to treat bitwise flags as bitwise flags. For example,: if (bfd->fid != -1 && (flags & O_RDONLY) == O_RDONLY) However, Robert's patch is correct to use: if (bfd->fid != -1 && flags == O_RDONLY) since flags is an 'access mode', and not bitwise values or'd together. Still, had it been properly treated as a convolution of bitwise values, it would not have resulted in a bug And, yes, the OS will cache the file with or without the FADV_DONTNEED call. It is a question of how long it will remain cached. With the FADV_DONTNEED call, each file is cached and then immediately released before the next file read. Without the FADV_DONTNEED, the OS will cache as many files as it possibly can. My problem with this approach is that it probably degrades performance for small files, particularly with many small files. It forces fdatasync and fadvise system calls after each file. If that is what is needed, then perhaps it is better to just open with O_SYNC | O_DIRECT and avoid caching in the first place. In any case, it is going to perform very differently backing up huge files that it does backing up many small files. So I agree with Robert that it should have a directive. > > Thanks, > Eric > > Le 2015-10-07 13:57, Robert Heinzmann <r.h...@fr...> a écrit : >> Hello, >> >> >> >> just a short followup. It seems I found the issue causing our file system cache to fill up during backup jobs and our virtual platform memory usage to increase. >> >> >> >> The O_RDONLY mask is “00” on Linux, casing the if flags & O_RDONLY to always fail. So the patch regarding posix_fadvise() in commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31 is actually dead code. (if (Anything & 00) { dead code; }) >> >> >> >> This diff fixes the issue: >> >> >> >> diff --git a/bacula/src/findlib/bfile.c b/bacula/src/findlib/bfile.c >> >> index e315675..29f04a3 100644 >> >> --- a/bacula/src/findlib/bfile.c >> >> +++ b/bacula/src/findlib/bfile.c >> >> @@ -997,9 +997,9 @@ int bopen(BFILE *bfd, const char *fname, int flags, mode_t mode) >> >> bfd->win32DecompContext.liNextHeader = 0; >> >> >> >> #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED) >> >> - if (bfd->fid != -1 && flags & O_RDONLY) { >> >> + if (bfd->fid != -1 && flags == O_RDONLY) { >> >> int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED); >> >> - Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat); >> >> + Dmsg2(400, "Did posix_fadvise POSIX_FADV_WILLNEED on %s stat=%d\n", fname, stat); >> >> } >> >> #endif >> >> >> >> @@ -1043,10 +1043,11 @@ int bclose(BFILE *bfd) >> >> return 0; >> >> } >> >> #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED) >> >> - if (bfd->m_flags & O_RDONLY) { >> >> + if (bfd->m_flags == O_RDONLY) { >> >> fdatasync(bfd->fid); /* sync the file */ >> >> /* Tell OS we don't need it any more */ >> >> posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED); >> >> + Dmsg1(400, "Did posix_fadvise POSIX_FADV_DONTNEED on File Desriptor %d\n", bfd->fid); >> >> } >> >> #endif >> >> >> >> After applying this patch, the cache memory does not increase a lot during backups on my test machine. Running widh bacula-fd –d 400 also shows the debugging messages. >> >> >> >> However this fix has a HUGE impact on behavior of existing backup jobs - altought it has huge benefits on virtual platforms with short backup windows also. >> >> >> >> Current behavior: >> >> - backup jobs with snapshots: caching of unneeded files in memory, filesystem cache is polluted, virtual platform memory usage increases >> >> - backup Jobs without snapshots: backup job warms up the cache of all files >> >> >> >> Behaviour with fix: >> >> - backup jobs with snapshots: only caching of 1 unneeded file in memory, filesystem cache is less polluted (largest file to backup), virtual platform memory usage decreases >> >> - backup Jobs without snapshots: backup job does not warms up the cache of all files, but also drops existing cache for cached files (may have performance impact for webservers, fileservers etc.) >> >> >> >> >> >> Conclusion: >> >> - As the impact on behavior is huge, fixing the bug in place can be problematic >> >> - Unfortunately the right way to impelement posix_fadvise() with POSIX_FADV_NOREUSE is not implemented on Linux (See http://lxr.free-electrons.com/source/mm/fadvise.c#L115), so there is “right” way to fix this issue >> >> - Best approach: there should be a Bacula Option to enable the POSIX_FADV_NOREUSE explicitly for selective bacula Jobs to allow usage of posix_fadvise() for direct I/O workloads (like mysql databases etc with snapshots etc.) and avoid it for webserver and fileserver workloads. Admins can decide. The default should be the current behavior to not call posix_fadvise(). Also the code path for SD/Dir und FD should be separate so enable this for the FD bfile.c code only (it seems currently it is the same code). >> >> >> >> Regards, >> >> Robert >> >> >> >> Robert Heinzmann >> >> r.h...@fr... >> >> IT-Operations >> >> Travian Games GmbH >> >> >> >> Von: Robert Heinzmann [mailto:r.h...@fr...] >> Gesendet: Mittwoch, 7. Oktober 2015 11:16 >> An: bac...@li... >> Betreff: [Bacula-users] Support for HAVE_POSIX_FADVISE on Bacula Client >> >> >> >> Hello, >> >> >> >> we are using bacula on a virtual plattform (> 600 clients) and investiate moderate swap usage issues / cache usage issues. It seems that during backup of mysql datafiles (LVM Snapshot), the “cached” memory usage of the server increases and thus bacula backup reads (only done one) are cached (… polluting the rest of the vfs cache ?) >> >> >> >> From reading the source of src/findlib/bfile.c, bacula supports posix_fadvise for a long time now (2007), by those two code pices: >> >> >> >> commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31 >> >> Author: Kern Sibbald <ke...@si...> >> >> Date: Thu May 24 19:58:07 2007 +0000 >> >> >> >> kes Add code to tell the OS that we no longer need a cached >> >> file that we were reading. In findlib/bfile.c. Also, >> >> only cache files that we are reading. >> >> kes Tweak to bsmtp to eliminate compiler warnings on Win32. >> >> kes Implement script to automatically generate cats and dll .def >> >> files for Win32 dll. >> >> kes Update README.mingw32 to include new .def file generation. >> >> >> >> git-svn-id: https://bacula.svn.sourceforge.net/svnroot/bacula/trunk@4898 91ce42f0-d328-0410-95d8-f526ca767f89 >> >> >> >> bopen() >> >> >> >> #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED) >> >> if (bfd->fid != -1 && flags & O_RDONLY) { >> >> int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED); >> >> Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat); >> >> } >> >> #endif >> >> >> >> bclose() >> >> >> >> #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED) >> >> if (bfd->m_flags & O_RDONLY) { >> >> fdatasync(bfd->fid); /* sync the file */ >> >> /* Tell OS we don't need it any more */ >> >> posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED); >> >> } >> >> #endif >> >> >> >> I checked that our bacula client was compiled with HAVE_POSIX_FADVISE (added debug messages). >> >> >> >> I also added more debugging code and it seems that during a normal backup run, the code is never executed on bacula-fd as O_RDONLY is not set in the flags of the originating bopen request. >> >> >> >> Is this a bug or a feature ? >> >> >> >> How can I prevent cache usage of bacula-fd on the client side at all ? Is there a Flag in bacula-fd to force O_READONLY opening of backed up files ? >> >> >> >> Regards, >> >> Robert > ------------------------------------------------------------------------------ > Full-scale, agent-less Infrastructure Monitoring from a single dashboard > Integrate with 40+ ManageEngine ITSM Solutions for complete visibility > Physical-Virtual-Cloud Infrastructure monitoring from one console > Real user monitoring with APM Insights and performance trend reports > Learn More http://pubads.g.doubleclick.net/gampad/clk?id=247754911&iu=/4140 > _______________________________________________ > Bacula-users mailing list > Bac...@li... > https://lists.sourceforge.net/lists/listinfo/bacula-users |
From: Robert H. <r.h...@fr...> - 2015-10-08 11:54:40
|
Hi, > I think you did a really nice investigation work :-) Thanks ... was fun :) > I think that your patch is almost right, and we need to figure if we want to add specific directives for that. I tend to believe right now that with or without the POSIX call, the os will cache the file, and it sounds better to do it in advance, let see what other people are thinking about your idea. the point is, does it cache the file and evict the cache after the backup of the file (what is with posix call) Example WITH fix: ---------------------- Server: 8 GB unused memory Running Application (aka workload) = 4 of working set and cached files Bacula backing up 800 x 100 MB FILE (8 GB in total) => After backup Running Application: If bacula backs up the working set data: 0 GB cached If bacula backs up other sets of data: 4 GB cached (e.g. a Snapshot of the dataset which also has new filehandles) <=== Our case !!!! Bacula 100MB of cached (and never reused) data OR does bacula polute the whole file system cache Example WITHOUT fix: ----------------------------- Server: 8 GB unused memory Running Application (aka workload) = 4 of working set and cached files Bacula backing up 800 x 100 MB FILE (8 GB in total) => After backup Running Application: 0 GB cached Bacula 8 GB of cached (and never reused) data Regards, Robert Robert Heinzmann r.h...@fr... IT-Operations Travian Games GmbH -----Ursprüngliche Nachricht----- Von: Eric Bollengier [mailto:eri...@ba...] Gesendet: Mittwoch, 7. Oktober 2015 16:50 An: Robert Heinzmann <r.h...@fr...> Cc: bac...@li... <bac...@li...> Betreff: Re: [Bacula-users] Support for HAVE_POSIX_FADVISE on Bacula Client Hello Robert, I think you did a really nice investigation work :-) I think that your patch is almost right, and we need to figure if we want to add specific directives for that. I tend to believe right now that with or without the POSIX call, the os will cache the file, and it sounds better to do it in advance, let see what other people are thinking about your idea. Thanks, Eric |
From: Eric B. <eri...@ba...> - 2015-10-09 06:33:06
|
Hello Robert, Le 08. 10. 15 13:54, Robert Heinzmann a écrit : > Hi, > >> I think you did a really nice investigation work :-) > Thanks ... was fun :) > >> I think that your patch is almost right, and we need to figure if we want to add specific directives for that. I tend to believe right now that with or without the POSIX call, the os will cache the file, and it sounds better to do it in advance, let see what other people are thinking about your idea. > > the point is, does it cache the file and evict the cache after the backup of the file (what is with posix call) > > Example WITH fix: > ---------------------- > > Server: 8 GB unused memory > Running Application (aka workload) = 4 of working set and cached files > Bacula backing up 800 x 100 MB FILE (8 GB in total) > > => After backup > > Running Application: > If bacula backs up the working set data: 0 GB cached > If bacula backs up other sets of data: 4 GB cached (e.g. a Snapshot of the dataset which also has new filehandles) <=== Our case !!!! > Bacula 100MB of cached (and never reused) data > > OR > > does bacula polute the whole file system cache > > Example WITHOUT fix: > ----------------------------- > > Server: 8 GB unused memory > Running Application (aka workload) = 4 of working set and cached files > Bacula backing up 800 x 100 MB FILE (8 GB in total) > > => After backup > > Running Application: 0 GB cached > Bacula 8 GB of cached (and never reused) data > > I think that if I understand correctly what you wrote, the "fix" is better in all cases (and I agree with you), so I'm not convinced that we should have a directive. Best Regards, Eric |
From: Kern S. <ke...@si...> - 2015-10-10 02:06:02
|
Hello, Thanks for pointing this out. I still have problems imagining that they would define O_RDONLY as 0 in a bitmapped variable!!! Unfortunately, the patch below will not work in all cases because O_RDONLY does not correspond to any status in the flags variable. Tomorrow, I will post a fix to this to the public git repository. Again, thanks for pointing out the problem and getting *very* close to a solution. Best regards, Kern On 10/07/2015 04:57 AM, Robert Heinzmann wrote: > > Hello, > > > > just a short followup. It seems I found the issue causing our file > system cache to fill up during backup jobs and our virtual platform > memory usage to increase. > > > > The O_RDONLY mask is “00” on Linux, casing the if flags & O_RDONLY to > always fail. So the patch regarding posix_fadvise() in commit > 4df486dd5b04cc16178efcb75eaf3dbd4be44b31 is actually dead code. (if > (Anything & 00) { dead code; }) > > > > This diff fixes the issue: > > > > diff --git a/bacula/src/findlib/bfile.c b/bacula/src/findlib/bfile.c > > index e315675..29f04a3 100644 > > --- a/bacula/src/findlib/bfile.c > > +++ b/bacula/src/findlib/bfile.c > > @@ -997,9 +997,9 @@ int bopen(BFILE *bfd, const char *fname, int > flags, mode_t mode) > > bfd->win32DecompContext.liNextHeader = 0; > > > > #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED) > > - if (bfd->fid != -1 && flags & O_RDONLY) { > > + if (bfd->fid != -1 && flags == O_RDONLY) { > > int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED); > > - Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat); > > + Dmsg2(400, "Did posix_fadvise POSIX_FADV_WILLNEED on %s > stat=%d\n", fname, stat); > > } > > #endif > > > > @@ -1043,10 +1043,11 @@ int bclose(BFILE *bfd) > > return 0; > > } > > #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED) > > - if (bfd->m_flags & O_RDONLY) { > > + if (bfd->m_flags == O_RDONLY) { > > fdatasync(bfd->fid); /* sync the file */ > > /* Tell OS we don't need it any more */ > > posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED); > > + Dmsg1(400, "Did posix_fadvise POSIX_FADV_DONTNEED on File > Desriptor %d\n", bfd->fid); > > } > > #endif > > > > After applying this patch, the cache memory does not increase a lot > during backups on my test machine. Running widh bacula-fd –d 400 also > shows the debugging messages. > > > > However this fix has a HUGE impact on behavior of existing backup jobs > - altought it has huge benefits on virtual platforms with short backup > windows also. > > > > Current behavior: > > - backup jobs with snapshots: caching of unneeded files in > memory, filesystem cache is polluted, virtual platform memory usage > increases > > - backup Jobs without snapshots: backup job warms up the > cache of all files > > > > Behaviour with fix: > > - backup jobs with snapshots: only caching of 1 unneeded file > in memory, filesystem cache is less polluted (largest file to backup), > virtual platform memory usage decreases > > - backup Jobs without snapshots: backup job does not warms up > the cache of all files, but also drops existing cache for cached files > (may have performance impact for webservers, fileservers etc.) > > > > > > Conclusion: > > - As the impact on behavior is huge, fixing the bug in place > can be problematic > > - Unfortunately the right way to impelement posix_fadvise() > with POSIX_FADV_NOREUSE is not implemented on Linux (See > http://lxr.free-electrons.com/source/mm/fadvise.c#L115), so there is > “right” way to fix this issue > > - Best approach: there should be a Bacula Option to enable > the POSIX_FADV_NOREUSE explicitly for selective bacula Jobs to allow > usage of posix_fadvise() for direct I/O workloads (like mysql > databases etc with snapshots etc.) and avoid it for webserver and > fileserver workloads. Admins can decide. The default should be the > current behavior to not call posix_fadvise(). Also the code path for > SD/Dir und FD should be separate so enable this for the FD bfile.c > code only (it seems currently it is the same code). > > > > Regards, > > Robert > > > > Robert Heinzmann > > _r....@fr..._ > > IT-Operations > > Travian Games GmbH > > > > *Von:*Robert Heinzmann [mailto:r.h...@fr...] > *Gesendet:* Mittwoch, 7. Oktober 2015 11:16 > *An:* bac...@li... > *Betreff:* [Bacula-users] Support for HAVE_POSIX_FADVISE on Bacula Client > > > > Hello, > > > > we are using bacula on a virtual plattform (> 600 clients) and > investiate moderate swap usage issues / cache usage issues. It seems > that during backup of mysql datafiles (LVM Snapshot), the “cached” > memory usage of the server increases and thus bacula backup reads > (only done one) are cached (… polluting the rest of the vfs cache ?) > > > > From reading the source of src/findlib/bfile.c, bacula supports > posix_fadvise for a long time now (2007), by those two code pices: > > > > commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31 > > Author: Kern Sibbald <ke...@si... <mailto:ke...@si...>> > > Date: Thu May 24 19:58:07 2007 +0000 > > > > kes Add code to tell the OS that we no longer need a cached > > file that we were reading. In findlib/bfile.c. Also, > > only cache files that we are reading. > > kes Tweak to bsmtp to eliminate compiler warnings on Win32. > > kes Implement script to automatically generate cats and dll .def > > files for Win32 dll. > > kes Update README.mingw32 to include new .def file generation. > > > > git-svn-id: > https://bacula.svn.sourceforge.net/svnroot/bacula/trunk@4898 > 91ce42f0-d328-0410-95d8-f526ca767f89 > > > > bopen() > > > > #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED) > > if (bfd->fid != -1 && flags & O_RDONLY) { > > int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED); > > Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat); > > } > > #endif > > > > bclose() > > > > #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED) > > if (bfd->m_flags & O_RDONLY) { > > fdatasync(bfd->fid); /* sync the file */ > > /* Tell OS we don't need it any more */ > > posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED); > > } > > #endif > > > > I checked that our bacula client was compiled with HAVE_POSIX_FADVISE > (added debug messages). > > > > I also added more debugging code and it seems that during a normal > backup run, the code is never executed on bacula-fd as O_RDONLY is not > set in the flags of the originating bopen request. > > > > Is this a bug or a feature ? > > > > How can I prevent cache usage of bacula-fd on the client side at all ? > Is there a Flag in bacula-fd to force O_READONLY opening of backed up > files ? > > > > Regards, > > Robert > > > > ------------------------------------------------------------------------------ > Full-scale, agent-less Infrastructure Monitoring from a single dashboard > Integrate with 40+ ManageEngine ITSM Solutions for complete visibility > Physical-Virtual-Cloud Infrastructure monitoring from one console > Real user monitoring with APM Insights and performance trend reports > Learn More http://pubads.g.doubleclick.net/gampad/clk?id=247754911&iu=/4140 > > > _______________________________________________ > Bacula-users mailing list > Bac...@li... > https://lists.sourceforge.net/lists/listinfo/bacula-users |
From: Kern S. <ke...@si...> - 2015-10-10 18:30:57
|
On 10/10/2015 10:16 AM, Phil Stracchino wrote: > On 10/09/15 22:05, Kern Sibbald wrote: >> Hello, >> >> Thanks for pointing this out. I still have problems imagining that they >> would define O_RDONLY as 0 in a bitmapped variable!!! > It makes more sense if you think of it as the absence of O_RDWR. Unfortunately, it is even more complicated than that, because it is actually the absence of O_RWRD and O_WRONLY. In my opinion the original implementation was faulty because O_RDONLY should really be defined as: #define O_RDONLY !(O_RWRD&O_RONLY) See my second to last commit ... Goan, but that is how it is so now we know, and hopefully it should now work correctly. Best regards, Kern |
From: Phil S. <ph...@ca...> - 2015-10-10 17:16:42
|
On 10/09/15 22:05, Kern Sibbald wrote: > Hello, > > Thanks for pointing this out. I still have problems imagining that they > would define O_RDONLY as 0 in a bitmapped variable!!! It makes more sense if you think of it as the absence of O_RDWR. -- Phil Stracchino Babylon Communications ph...@ca... ph...@co... Landline: 603.293.8485 |
From: Richard H. <Ric...@in...> - 2015-10-10 22:10:38
|
On 10/10/2015 19:30, Kern Sibbald wrote: > On 10/10/2015 10:16 AM, Phil Stracchino wrote: >> On 10/09/15 22:05, Kern Sibbald wrote: >>> Hello, >>> >>> Thanks for pointing this out. I still have problems imagining that they >>> would define O_RDONLY as 0 in a bitmapped variable!!! >> It makes more sense if you think of it as the absence of O_RDWR. > > Unfortunately, it is even more complicated than that, because it is > actually the absence of O_RWRD and O_WRONLY. > > In my opinion the original implementation was faulty because O_RDONLY > should really be defined as: > > #define O_RDONLY !(O_RWRD&O_RONLY) > > See my second to last commit ... > > Goan, but that is how it is so now we know, and hopefully it should now > work correctly. > > Best regards, > Kern Aren't people over-complicating this? You need to think of this as a 2-bit integer field, not as a bitmap. Then looking at the constants defined in fcntl.h, it becomes clear (to me, anyway!) that the code should be (sorry, I forget the exact context, but you'll see what I mean, I hope) (flags & O_ACCMODE) == O_RDONLY (The Solaris box I am looking at defines #define O_ACCMODE 3 /* Mask for file access modes */ while a nearby Linux system has #define O_ACCMODE 00000003 #define O_RDONLY 00000000 #define O_WRONLY 00000001 #define O_RDWR 00000002 ) Regards, Richard |
From: Kern S. <ke...@si...> - 2015-10-11 02:15:44
|
Hello, From what I read on the web, this whole thing is a bit of a mess, and apparently POSIX requires that O_ACCMODE not be 3 because there are at least two bits besides O_RWRD and O_RDONLY. However, on most systems O_ACCMODE is 3. It would be hard to consider any part of flags as an integer because it is a bit field, and on Linux (and most other systems), the value of the "integer" produced by anding with O_ACMODE cannot take on the value 3 (or more precisely, the value of 3 is illogical). In any case, I have fixed the current Bacula code to work correctly with all the modes that Bacula currently uses. I thank Robert Heinzmann for finding and identifying the problem :-) Best regards, Kern On 10/10/2015 02:44 PM, Richard Hall wrote: > On 10/10/2015 19:30, Kern Sibbald wrote: >> On 10/10/2015 10:16 AM, Phil Stracchino wrote: >>> On 10/09/15 22:05, Kern Sibbald wrote: >>>> Hello, >>>> >>>> Thanks for pointing this out. I still have problems imagining that they >>>> would define O_RDONLY as 0 in a bitmapped variable!!! >>> It makes more sense if you think of it as the absence of O_RDWR. >> Unfortunately, it is even more complicated than that, because it is >> actually the absence of O_RWRD and O_WRONLY. >> >> In my opinion the original implementation was faulty because O_RDONLY >> should really be defined as: >> >> #define O_RDONLY !(O_RWRD&O_RONLY) >> >> See my second to last commit ... >> >> Goan, but that is how it is so now we know, and hopefully it should now >> work correctly. >> >> Best regards, >> Kern > Aren't people over-complicating this? You need to think of this as a > 2-bit integer field, not as a bitmap. Then looking at the constants > defined in fcntl.h, it becomes clear (to me, anyway!) that the code > should be (sorry, I forget the exact context, but you'll see what I > mean, I hope) > > (flags & O_ACCMODE) == O_RDONLY > > (The Solaris box I am looking at defines > > #define O_ACCMODE 3 /* Mask for file access modes */ > > while a nearby Linux system has > > #define O_ACCMODE 00000003 > #define O_RDONLY 00000000 > #define O_WRONLY 00000001 > #define O_RDWR 00000002 > > ) > > Regards, > Richard > > ------------------------------------------------------------------------------ > _______________________________________________ > Bacula-users mailing list > Bac...@li... > https://lists.sourceforge.net/lists/listinfo/bacula-users > a |
From: Josh F. <jf...@pv...> - 2015-10-12 14:39:16
|
On 10/10/2015 10:15 PM, Kern Sibbald wrote: > Hello, > > >From what I read on the web, this whole thing is a bit of a mess, and > apparently POSIX requires that O_ACCMODE not be 3 because there are at > least two bits besides O_RWRD and O_RDONLY. However, on most systems > O_ACCMODE is 3. It would be hard to consider any part of flags as an > integer because it is a bit field, and on Linux (and most other > systems), the value of the "integer" produced by anding with O_ACMODE > cannot take on the value 3 (or more precisely, the value of 3 is illogical). Well, according to the open(2) man page from RHEL 7, the two least significant bits are NOT part of the bitfield: Unlike the other values that can be specified in flags, the access mode values O_RDONLY, O_WRONLY, and O_RDWR, do not specify individual bits. Rather, they define the low order two bits of flags, and are defined respectively as 0, 1, and 2. Granted, it is strange and probably a holdover from long ago when memory was scarce and CPU register sizes were small. . > > In any case, I have fixed the current Bacula code to work correctly with > all the modes that Bacula currently uses. > > I thank Robert Heinzmann for finding and identifying the problem :-) > > Best regards, > Kern > > > > On 10/10/2015 02:44 PM, Richard Hall wrote: >> On 10/10/2015 19:30, Kern Sibbald wrote: >>> On 10/10/2015 10:16 AM, Phil Stracchino wrote: >>>> On 10/09/15 22:05, Kern Sibbald wrote: >>>>> Hello, >>>>> >>>>> Thanks for pointing this out. I still have problems imagining that they >>>>> would define O_RDONLY as 0 in a bitmapped variable!!! >>>> It makes more sense if you think of it as the absence of O_RDWR. >>> Unfortunately, it is even more complicated than that, because it is >>> actually the absence of O_RWRD and O_WRONLY. >>> >>> In my opinion the original implementation was faulty because O_RDONLY >>> should really be defined as: >>> >>> #define O_RDONLY !(O_RWRD&O_RONLY) >>> >>> See my second to last commit ... >>> >>> Goan, but that is how it is so now we know, and hopefully it should now >>> work correctly. >>> >>> Best regards, >>> Kern >> Aren't people over-complicating this? You need to think of this as a >> 2-bit integer field, not as a bitmap. Then looking at the constants >> defined in fcntl.h, it becomes clear (to me, anyway!) that the code >> should be (sorry, I forget the exact context, but you'll see what I >> mean, I hope) >> >> (flags & O_ACCMODE) == O_RDONLY >> >> (The Solaris box I am looking at defines >> >> #define O_ACCMODE 3 /* Mask for file access modes */ >> >> while a nearby Linux system has >> >> #define O_ACCMODE 00000003 >> #define O_RDONLY 00000000 >> #define O_WRONLY 00000001 >> #define O_RDWR 00000002 >> >> ) >> >> Regards, >> Richard >> >> ------------------------------------------------------------------------------ >> _______________________________________________ >> Bacula-users mailing list >> Bac...@li... >> https://lists.sourceforge.net/lists/listinfo/bacula-users >> > a > > > ------------------------------------------------------------------------------ > _______________________________________________ > Bacula-users mailing list > Bac...@li... > https://lists.sourceforge.net/lists/listinfo/bacula-users |