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
>
> [email protected]_
>
> IT-Operations
>
> Travian Games GmbH
>
>  
>
> *Von:*Robert Heinzmann [mailto:[email protected]]
> *Gesendet:* Mittwoch, 7. Oktober 2015 11:16
> *An:* [email protected]
> *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 <[email protected] <mailto:[email protected]>>
>
> 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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/bacula-users

------------------------------------------------------------------------------
_______________________________________________
Bacula-users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bacula-users

Reply via email to