Re: [Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-04-04 Thread Phil Dibowitz
* added docs * stopped destroying the FDs on sync * changed the name of the macro to be prefixed with an `_` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub:

Re: [Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-04-04 Thread Phil Dibowitz
@n3npq Uh, I am doing it in lib/fsm.h not in Fclose(), I dunno what you are talking about. Yes, the fsync() is super expensive on rotation media, as mentioned, don't use it in those cases. As I mentioned, I don't have good numbers on the positive effects, only that our databases stall far

Re: [Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-04-04 Thread Phil Dibowitz
jaymzh commented on this pull request. > @@ -231,6 +232,9 @@ static int expandRegular(rpmfi fi, const char *dest, > rpmpsm psm, int nodigest, i exit: if (wfd) { int myerrno = errno; +if (rpmExpandNumeric("%{force_fsync_on_close}")) { `oneshot` is processed in

Re: [Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-04-04 Thread Phil Dibowitz
jaymzh commented on this pull request. > @@ -1116,6 +1116,28 @@ int Fseek(FD_t fd, off_t offset, int whence) return rc; } +int Fsync(FD_t fd) +{ +int rc = 0, ec = 0; + +if (fd == NULL) + return -1; + +fd = fdLink(fd); +for (FDSTACK_t fps = fd->fps; fps != NULL; fps

Re: [Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-04-04 Thread Phil Dibowitz
As to the name, the implementation has significant implications, and so I think the name should be kept as-is... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub:

Re: [Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-04-04 Thread Phil Dibowitz
Ah yes, good call @Conan-Kudo - will add that tomorrow. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub:

Re: [Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-04-06 Thread Phil Dibowitz
@n3npq as I called out a few times... the examples I found were all in the build/ directory... maybe I'm just not seeing another good example. Moving to mmap() and madvise() is a major invasive change... if someone wants to take that on, awesome, but this is a simple solution that buys us a

Re: [Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-04-06 Thread Phil Dibowitz
To your other question we're still trying to come up with numbers, the system has many things going on and showing something 100% attributable to this is hard - I'm trying to see if we can get a more exact A/B test. madvise() doesn't help ensure there's less dirty data in the system, which is

Re: [Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-04-05 Thread Phil Dibowitz
@pmatilai - I'm still looking for a good suggestion on where to read that macro and store it in a static variable. @pmatilai - I'm ok with fsync_periodically or some such, it just means that if we ever want to change it, we have no way to do it nicely... we can't make a new `%_fsync_on_mb 50`

Re: [Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-04-05 Thread Phil Dibowitz
jaymzh commented on this pull request. > +int rc = 0, ec = 0; + +if (fd == NULL) + return -1; + +fd = fdLink(fd); +for (FDSTACK_t fps = fd->fps; fps != NULL; fps = fps->prev) { + if (fps->fdno >= 0) { +rc = fsync(fps->fdno); + if (ec == 0 && rc)

Re: [Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-04-14 Thread Phil Dibowitz
@cgwalters - Jens works here... yes, the problem description is related, but while a system-wide solution can make this better, being able to administratively choose things that can never ever full the buffer is still important. -- You are receiving this because you are subscribed to this

Re: [Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-04-13 Thread Phil Dibowitz
In our case ~ 1MB is what we want. This will never be a good idea for rotating disks... and also most people will never need this. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub:

[Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-04-03 Thread Phil Dibowitz
This patch adds a new macro that has rpm call fsync() before each close(). The feature is off by default since most people will not want this, and is turned on either with a macro or command-line flag. There are a few reasons for wanting this: 1. To ensure that an RPM installation never fulls IO

Re: [Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-04-07 Thread Phil Dibowitz
OK, so here's a few different takes on numbers. First graphs. Graph one is with the macro on: https://g.defau.lt/#gluSzlb39ScnZU4FS2AtrYUF31IYct Graph2 is with the macro off: https://g.defau.lt/#6We3VgDfcXi7zkhMMzdesQLKD5iMUd The colors are reads, read bytes, writes, write bytes. You can see

Re: [Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-04-17 Thread Phil Dibowitz
@pmatilai - I'd like to get something merged sooner rather than later. There are two options I see - (1) my patch (plus moving the parsing of the file into some static variable) or (2) @n3npq patch, but turning it off by default. Those are the two safe, clean, easy methods going forward. Can

Re: [Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-04-14 Thread Phil Dibowitz
In regards to your questions an optimal implementation can come whenever, I'd like to get something merged nowish that I can opt-in to on hosts that t need it. So changing your patch to default to off, and then leaving it at the sync rate its at is fine. 1MB is probably nicer to the disk but

Re: [Rpm-maint] [rpm-software-management/rpm] Add macro to force fsync() on close() (#187)

2017-06-01 Thread Phil Dibowitz
Updated this PR to use a static variable. I'm still fine with either solution, but @pmatilai and @megaumi @ffesti - can someone weigh in, please? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub:

Re: [Rpm-maint] [rpm-software-management/rpm] Rpm query causes corruption in the file-backed mmaped bdb regions (#232)

2018-02-13 Thread Phil Dibowitz
On a related note... our fix-the-rpm-db program has been opensourced: https://github.com/facebookincubator/dcrpm We run it every 15 minutes as a pre-script to configuration management. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it

Re: [Rpm-maint] [rpm-software-management/rpm] Rpm query causes corruption in the file-backed mmaped bdb regions (#232)

2018-02-13 Thread Phil Dibowitz
Note that we don't run db_recover unless we detect an issue. dcrpm works by trying to detect common issues and issue the nicest possible recovery - from db_recover to other finding held locks by bad actors to other things. -- You are receiving this because you are subscribed to this thread.

Re: [Rpm-maint] [rpm-software-management/rpm] Rpm query causes corruption in the file-backed mmaped bdb regions (#232)

2018-02-14 Thread Phil Dibowitz
Hence this Issue was filed. :) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/232#issuecomment-365676303___ Rpm-maint mailing

Re: [Rpm-maint] [rpm-software-management/rpm] Rpm query causes corruption in the file-backed mmaped bdb regions (#232)

2018-02-14 Thread Phil Dibowitz
I'd be very surprised if this wasn't a cause of many of our problems in Linux too. I suspect a common case is someone querying the DB (`rpm -qa`) when an transaction happens, and then theoretically-read-only operation ends up writing stale data back, but I haven't actually had the time to sit

Re: [Rpm-maint] [rpm-software-management/rpm] Rpm query causes corruption in the file-backed mmaped bdb regions (#232)

2019-11-21 Thread Phil Dibowitz
@jianwei1216 Are the processes running as root? If so, the memmap() the DB read-write, and write it back out when they are done... hence if then something changes at the same time, one of those process can write back old pages. -- You are receiving this because you are subscribed to this

Re: [Rpm-maint] [rpm-software-management/rpm] Rpm query causes corruption in the file-backed mmaped bdb regions (#232)

2019-11-21 Thread Phil Dibowitz
You could instead run it as nonroot: `su -c "rpm -q " nobody` (or possibly `su -c "rpm -q " -s /bin/bash nobody` if nobody's shell is not a real shell). Or you can make sure [dcrpm](https://github.com/facebookincubator/dcrpm) runs in a regular basis to detect and correct your RPM db... but