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.heinzmann AT freelancer.traviangames
> DOT com> 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.heinzmann AT freelancer.traviangames DOT com
>>
>> IT-Operations
>>
>> Travian Games GmbH
>>
>>
>>
>> Von: Robert Heinzmann [mailto:r.heinzmann AT freelancer.traviangames DOT com]
>> Gesendet: Mittwoch, 7. Oktober 2015 11:16
>> An: bacula-users AT lists.sourceforge DOT net
>> 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 <kern AT sibbald DOT com>
>>
>> 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
> Bacula-users AT lists.sourceforge DOT net
> https://lists.sourceforge.net/lists/listinfo/bacula-users
------------------------------------------------------------------------------
_______________________________________________
Bacula-users mailing list
Bacula-users AT lists.sourceforge DOT net
https://lists.sourceforge.net/lists/listinfo/bacula-users
|
