Re: [HACKERS] LWLock Queue Jumping
Greg Stark wrote: > On Fri, Aug 28, 2009 at 8:07 PM, Simon Riggs wrote: >> WALInsertLock is heavily contended and likely always will be even if we >> apply some of the planned fixes. > > I've lost any earlier messages, could you resend the raw data on which > this is based? I don't have any pointers right now, but WALInsertLock does often show up as a bottleneck in write-intensive benchmarks. >> Some callers of WALInsertLock are more important than others >> >> * Writing new Clog or Multixact pages (serialized by ClogControlLock) >> * For Hot Standby, writing SnapshotData (serialized by ProcArrayLock) >> >> In these cases it seems like we can skip straight to the front of the >> WALInsertLock queue without problem. > > Reordering some exclusive lockers ahead of other exclusive lockers > won't reduce the amount of time the lock is held at all. Are you > saying the reason to do it is to reduce time spent waiting on this > lock while holding other critical locks? That's what I thought. I don't know about the clog/multixact issue, it doesn't seem like it would be too bad, given how seldom new clog or multixact pages are written. The Hot Standby thing has been discussed, but no-one has actually posted a patch which does the locking correctly, where the ProcArrayLock is held while the SnapshotData WAL record is inserted. So there is no evidence that it's actually a problem, we might be making a mountain out of a molehill. It will have practically no effect on throughput, given how seldom SnapshotData records are written (once per checkpoint), but if it causes a significant bump to response times, that might be a problem. This is a good idea to keep in mind, but right now it feels like a solution in search of a problem. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] LWLock Queue Jumping
On Fri, Aug 28, 2009 at 8:07 PM, Simon Riggs wrote: > WALInsertLock is heavily contended and likely always will be even if we > apply some of the planned fixes. I've lost any earlier messages, could you resend the raw data on which this is based? > Some callers of WALInsertLock are more important than others > > * Writing new Clog or Multixact pages (serialized by ClogControlLock) > * For Hot Standby, writing SnapshotData (serialized by ProcArrayLock) > > In these cases it seems like we can skip straight to the front of the > WALInsertLock queue without problem. How does re-ordering reduce the contention? We reorder shared lockers ahead of exclusive lockers because they can all hold the lock at the same time so we can reduce the amount of time the lock is held. Reordering some exclusive lockers ahead of other exclusive lockers won't reduce the amount of time the lock is held at all. Are you saying the reason to do it is to reduce time spent waiting on this lock while holding other critical locks? Do we have tools to measure how long is being spent waiting on one lock while holding another lock so we can see if there's a problem and whether this helps? -- greg http://mit.edu/~gsstark/resume.pdf -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Memory context usage
Tom Lane escreveu: Adriano Lange writes: I need to control the size of a memory context on the fly and take some actions when the used memory exceeds a defined size. The existing places that do that sort of thing do their own counting of how much they've allocated. I have seen that the AllocSet structure is hid by MemoryContext which has only a stderr output interface for its stats. I would need these stats internally but maybe this is only my demand for now. Thanks for attention. Adriano -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] LWLock Queue Jumping
On Sat, Aug 29, 2009 at 4:02 AM, Simon Riggs wrote: > > On Fri, 2009-08-28 at 14:44 -0700, Jeff Janes wrote: > > > I'd previously implemented this just by copying and pasting and making > > some changes, perhaps not the most desirable way but I thought adding > > another parameter to all existing invocations would be a bit > > excessive. > > That's the way I would implement it also, but would call it > LWLockAcquireWithPriority() so that it's purpose is clear, rather than > refer to its implementation, which may change. Yes, good idea. But thinking about it as a patch to be applied, rather than a proof of concept, I think the best solution would be to add a third argument (boolean Priority) to LWLockAquire, and hunt down all existing invocations and change them to include false as the extra argument. Copying 160 lines of code to change 4 of them in the copy is temporarily easier, but not a good solution for the long term. > > I've tested it fairly thoroughly, > > Please send the tested patch, if this isn't it. What tests were made? I'd have a hard time coming up with the full origianl patch, as my changes for files other than lwlock.c were blown away by parallel efforts and an rsync to the repo. The above was just an exploratory tool, not proposed as an actual patch to be applied to HEAD. If we want to add a parameter to the existing LWLockAcquire, I'll work on coming up with a tested patch for that. My testing was to run the normal regression test (which often failed to detect my early buggy implementations), then load testing with pgbench (which always (that I know of) found them when -c > 1) and a custom Perl script I use. Since WALWriteLock is heavily used and contended under pgbench -c 20, and lwlock is agnostic to the exact identity of the underlying lock, I think this test was pretty thorough for the implementation. But not of course for starvation issues, which would have to be tested on a case by case basis when a specific Acquire invocation is changed to be high priority. If you have ideas for other tests to do, or corner cases that are likely to be overlooked by my tests, I'll try to work tests for them in too. > > > > in the context of using it in AdvanceXLInsertBuffer for acquiring the > > WALWriteLock. > > Apologies if you'd already suggested that recently. I read a few of your > posts but not all of them. > I don't think WALWriteLock from AdvanceXLInsertBuffer is an important > area, but I don't see any harm from it either. I had not mentioned it before. The change helped by ~50% or so when wal_buffers was undersized (kept at the default setting) but did not help significantly when wal_buffers was generously sized. I didn't think we would be interested in introducing a new locking procedure just to optimize performance for a poorly configured server. But if we are to introduce this method for other reasons, I think it should be used for AdvanceXLInsertBuffer as well. Cheers, Jeff
Re: [HACKERS] WIP: remove use of flat auth file for client authentication
On Sat, 2009-08-29 at 11:19 -0400, Tom Lane wrote: > Simon Riggs writes: > > Specifically, should I remove the parts of the HS patch that refresh > > those files? > > Yes. This was the last part that I was afraid might have insurmountable > problems. There are some bits yet to do but they're in the nature of > crank-turning, I believe. I think it's a good bet the flat files will > be gone before the next commitfest starts. OK, its gone. Shame really, took a while to make that work. :) > BTW, one of the to-do bits is to find another way to initialize the > XID wrap limit at database start. Right now that's set up as a > side-effect of scanning pg_database to create the flat DB file. > The idea I had about it was to store the current wrap limit in > checkpoint WAL records and use the most recent checkpoint's value > as the initial limit. Can you tell me what you're doing about the > wrap limit in HS and whether that solution would be an issue for you? ...quakes in boots... We're weren't doing anything at all. I guess we will be now; thanks for alerting me. In general we don't look at the actual xid values themselves, so going past the xid max value itself shouldn't present an issue (that I can see, but I will think longer on this). A problem could occur if a standby query/snapshot lived long enough to see the same xid in different epochs. Realistically that's a long time and a query would be unlikely to survive that long without having first constrained the flow of cleaning records by holding a snapshot open on the primary. However, it could present a problem to someone and agree we must have a clear remedial action. The standard remedial action is to kill queries that cause conflicts, so we would need to kill queries based upon a wrap test. We could do that as each xid arrives, but it would be best to do that via something rare like checkpoint records, so that works for me. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: remove use of flat auth file for client authentication
On Sat, 2009-08-29 at 09:00 -0400, Stephen Frost wrote: > * Simon Riggs (si...@2ndquadrant.com) wrote: > > I get the feeling that part of the inspiration for this is that Hot > > Standby must maintain this file. If not, I'm curious as to the reasons > > for doing this. No objections however, just curiosity. > > The impetus for these changes was the performance complaint that adding > new users is extremely expensive once the files get to be large. > Solving that kind of a problem typically involves moving away from flat > files and to a database. Thankfully, we happen to have a database > already built-in. ;) LOL. Quite lucky that. :) -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] typo in doc/src/sgml/release-8.4.sgml
Alvaro Herrera wrote: > Jan Urba?ski wrote: > > Patch -p1 attached. > > Applied, thanks Backpatched to 8.4.X, which is the only backbranch where this fix is needed. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 8.5 release timetable, again
Tom Lane wrote: > "Kevin Grittner" writes: > > Robert Haas wrote: > >> The final CommitFest began November 11, 2008. It closed March 25, > >> 2009 (+ 144 days). Beta1 was released April 15, 2009 (+ 21 days). > > > I'm not entirely clear on what was happening during the 21 days > > between the end of the CommitFest and and the release of beta1. > > Release note drafting is one part of it, but mostly it's "loose end > cleanup". Historically there have always been a pile of loose ends > to be dealt with, and the CommitFest structure doesn't really do > anything to avoid that. If you're interested, attached are all the > commits between commitfest closure (which I announced immediately > after committing the addition of contrib/btree_gin) and stamping > beta1 (which was actually several days before the date Robert gives, > because of the need for the packagers to do their thing). > > It appears to me that release notes weren't actually the bottleneck this > time, though they have been in some prior cycles. Bruce committed a > first draft immediately after the commitfest closed. Rather, it was > arguing about things like \df behavior and cardinality() that took up > the time. Yep, the bottom line here is that patches get into CVS, but issues come up related to the patch, and we keep looking for good fixes, but once the final commit-fest is over, we _have_ to fix these issues. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 8.5 release timetable, again
Robert Haas wrote: > Both committers and non-committers are currently suffering from the > fact that there is not a lot of time in the year which is set aside > for development, i.e. neither CommitFest-time nor beta-time. To fix > this problem, we can: > > 1. Make CommitFests shorter. > 2. Make CommitFests less frequent. > 3. Continue developing during CommitFests. > 4. Make beta cycles shorter. > 5. Make beta cycles less frequent (i.e. lengthen the release cycle). > 6. Continue developing during beta. > > I believe (1) to be completely impractical and (3) to be > self-defeating. I suspect (2) will backfire badly. That doesn't > leave us with a lot of options. We can certainly do (5), but the > downside is that features that get committed won't hit release for a > very long time. I and others have suggested a couple of possible > approaches toward (4) or (6), such as changing the way we do release > notes, adding more regression tests to give us more (not perfect) > confidence that the release is solid, and/or branching the tree during > beta. None of those ideas have gotten a single vote of confidence > from you or Bruce. What's your suggestion? Another solution would be to make major releases less frequent. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] clang's static checker report.
Oh, I think I see what's happening. Our assertions can still be turned off at run-time with the variable assert_enabled. Hm, you would have to replace assert_enabled with a #define in postgres.h and then do something about the guc.c code which assigns to it. On another note is there any way to marke MemoryContextAlloc, MemoryContextAllocZero, palloc, repalloc, and friends as never returning NULL? I think that's causing most of the "null dereferenced" errors. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] clang's static checker report.
On 29 Aug 2009, at 17:35, Greg Stark wrote: We still have things like this showing "division by zero": Assert(activeTapes > 0); 1913 slotsPerTape = (state->memtupsize - state->mergefirstfree) / activeTapes; It looks like if you marked ExceptionalCondition() as never returning then it should hide this. well, it is marked as such , here's excerpt from differences to head: extern int ExceptionalCondition(const char *conditionName, const char *errorType, -const char *fileName, int lineNumber); +const char *fileName, int lineNumber) __attribute__((analyzer_noreturn)); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] clang's static checker report.
We still have things like this showing "division by zero": Assert(activeTapes > 0); 1913slotsPerTape = (state->memtupsize - state->mergefirstfree) / activeTapes; It looks like if you marked ExceptionalCondition() as never returning then it should hide this. -- greg http://mit.edu/~gsstark/resume.pdf -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] clang's static checker report.
new one at http://zlew.org/postgresql_static_check/scan-build-2009-08-29-3/ archive at : http://zlew.org/postgresql_static_check/postgresql_static_check_29thAugust2009.tar.xz as always, comments are welcomed. And constructive explanation of any wrong-results will be relayed to clang-checker developer(s). hth. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: remove use of flat auth file for client authentication
Simon Riggs writes: > Specifically, should I remove the parts of the HS patch that refresh > those files? Yes. This was the last part that I was afraid might have insurmountable problems. There are some bits yet to do but they're in the nature of crank-turning, I believe. I think it's a good bet the flat files will be gone before the next commitfest starts. BTW, one of the to-do bits is to find another way to initialize the XID wrap limit at database start. Right now that's set up as a side-effect of scanning pg_database to create the flat DB file. The idea I had about it was to store the current wrap limit in checkpoint WAL records and use the most recent checkpoint's value as the initial limit. Can you tell me what you're doing about the wrap limit in HS and whether that solution would be an issue for you? >> I suspect that >> this means some things are actively broken during InitPostgres's >> initial >> transaction --- for example, if it happens to try to take a lock that >> completes a deadlock cycle, the deadlock won't be detected because the >> lock timeout SIGALRM interrupt will never occur. Another example is >> that SI inval messaging isn't working during InitPostgres either. > Are we doing anything in the initial transaction that *could* deadlock, Sure --- we have to take locks to examine system catalogs. Pretty weak locks mind you, but there's certainly a potential there if someone else is doing something like a VACUUM FULL on the catalogs. > or cause an SI inval message? I don't think InitPostgres could *cause* a SI inval, I was more worried about whether it would fail to respond to them. The worst case result is probably a failure of the InitPostgres transaction to notice someone else's relfilenode reassignment on a system catalog, which would end up with a "failed to open file" kind of error. Again, the probabilities aren't very high, but they appear nonzero. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: remove use of flat auth file for client authentication
Greg Stark writes: > On Sat, Aug 29, 2009 at 6:00 AM, Tom Lane wrote: >> ... I didn't yet do anything >> about the idea of falling back to connecting to "postgres" when the >> specified target DB doesn't exist, but other than that small change >> I think it's about ready to go. > Falling back to connecting to "postgres" seems unnecessarily complex to me. If that's the consensus I don't have a problem with it ... it's certainly something we could add later if anyone complains. >> Another interesting point is that for this to work, those signal >> interrupts have to actually be enabled (doh) ... and up to now we have >> been running InitPostgres with most signals disabled. I suspect that >> this means some things are actively broken during InitPostgres's initial >> transaction --- for example, if it happens to try to take a lock that >> completes a deadlock cycle, the deadlock won't be detected because the >> lock timeout SIGALRM interrupt will never occur. Another example is >> that SI inval messaging isn't working during InitPostgres either. >> The patch addresses this by moving up PostgresMain's >> PG_SETMASK(&UnBlockSig); call to before InitPostgres. We might need to >> back-patch that bit, though I'm hesitant to fool with such a thing in >> back branches. > The deadlock can only fail to be detected by someone else if the whole > initpostgres thing takes longer than deadlock_timout I think. So it > doesn't seem very likely. Not sure how likely problems due to missed > SI messages are. The problem I was worried about was where InitPostgres tries to take the last lock in a deadlock cycle, and all the other participants have already run their deadlock checks and are just waiting. The InitPostgres transaction needs to run the checker to get out of this state, and it won't because it's got SIGALRM blocked. I agree that this is not very probable because InitPostgres takes only pretty weak locks, but I suspect that a failure scenario could be demonstrated. We have seen occasional reports of apparent undetected-deadlock situations, but of course I've got no proof that this is the cause. On the whole, I don't want to risk tinkering with it in the back branches, but I'm happy that it will be fixed going forward. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: remove use of flat auth file for client authentication
* Simon Riggs (si...@2ndquadrant.com) wrote: > I get the feeling that part of the inspiration for this is that Hot > Standby must maintain this file. If not, I'm curious as to the reasons > for doing this. No objections however, just curiosity. The impetus for these changes was the performance complaint that adding new users is extremely expensive once the files get to be large. Solving that kind of a problem typically involves moving away from flat files and to a database. Thankfully, we happen to have a database already built-in. ;) > Specifically, should I remove the parts of the HS patch that refresh > those files? I'd probably wait till it's committed and then rip it out as part of re-baseing against CVS. That's just my 2c on it and I've no clue how invasive the changes to HS are to do that. If it's easy to separate the changes and remove/add them as needed, then perhaps it doesn't matter if you do that now or wait. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] LWLock Queue Jumping
On Fri, 2009-08-28 at 14:44 -0700, Jeff Janes wrote: > I'd previously implemented this just by copying and pasting and making > some changes, perhaps not the most desirable way but I thought adding > another parameter to all existing invocations would be a bit > excessive. That's the way I would implement it also, but would call it LWLockAcquireWithPriority() so that it's purpose is clear, rather than refer to its implementation, which may change. > I've tested it fairly thoroughly, Please send the tested patch, if this isn't it. What tests were made? > in the context of using it in AdvanceXLInsertBuffer for acquiring the > WALWriteLock. Apologies if you'd already suggested that recently. I read a few of your posts but not all of them. I don't think WALWriteLock from AdvanceXLInsertBuffer is an important area, but I don't see any harm from it either. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: remove use of flat auth file for client authentication
On Sat, 2009-08-29 at 01:00 -0400, Tom Lane wrote: > Attached is a patch that removes the use of the flat auth file during > client authentication, instead using regular access to the pg_auth > catalogs. As previously discussed, this implies pushing the > authentication work down to InitPostgres. I didn't yet do anything > about the idea of falling back to connecting to "postgres" when the > specified target DB doesn't exist, but other than that small change > I think it's about ready to go. I get the feeling that part of the inspiration for this is that Hot Standby must maintain this file. If not, I'm curious as to the reasons for doing this. No objections however, just curiosity. Specifically, should I remove the parts of the HS patch that refresh those files? > I suspect that > this means some things are actively broken during InitPostgres's > initial > transaction --- for example, if it happens to try to take a lock that > completes a deadlock cycle, the deadlock won't be detected because the > lock timeout SIGALRM interrupt will never occur. Another example is > that SI inval messaging isn't working during InitPostgres either. Are we doing anything in the initial transaction that *could* deadlock, or cause an SI inval message? -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [pgsql-hackers] Daily digest v1.9418 (15 messages)
On Thu, Aug 27, 2009 at 01:12:20PM -0400, Robert Haas wrote: > This is pretty cool, IMO. Admittedly, it does seem hard to bottle it, > but you managed it, so it's not completely impossible. What you could > for this kind of thing is a series of patches and driver scripts, so > you build PostgreSQL with the patch, then run the driver script > against it. Probably we'd want to standardize some kind of framework > for the driver scripts, once we had a list of ideas for testing and > some idea what it should look like. Another similar idea I've had in the back of my head for a while is to setup postgres so it is the only process in a VM. Subsequently after every single write() syscall, snapshot the filesystem and then run the recovery process over each one. It would likely take an unbeleivably long time to run, and maybe there's some trick to speed it up, but together with code coverage results it could give you good results as to the reliability of the recovery process. Probably more a research project than anything else though. Have a nice day, -- Martijn van Oosterhout http://svana.org/kleptog/ > Please line up in a tree and maintain the heap invariant while > boarding. Thank you for flying nlogn airlines. signature.asc Description: Digital signature
Re: [HACKERS] WIP: remove use of flat auth file for client authentication
On Sat, Aug 29, 2009 at 6:00 AM, Tom Lane wrote: > Attached is a patch that removes the use of the flat auth file during > client authentication, instead using regular access to the pg_auth > catalogs. As previously discussed, this implies pushing the > authentication work down to InitPostgres. I didn't yet do anything > about the idea of falling back to connecting to "postgres" when the > specified target DB doesn't exist, but other than that small change > I think it's about ready to go. Falling back to connecting to "postgres" seems unnecessarily complex to me. > Another interesting point is that for this to work, those signal > interrupts have to actually be enabled (doh) ... and up to now we have > been running InitPostgres with most signals disabled. I suspect that > this means some things are actively broken during InitPostgres's initial > transaction --- for example, if it happens to try to take a lock that > completes a deadlock cycle, the deadlock won't be detected because the > lock timeout SIGALRM interrupt will never occur. Another example is > that SI inval messaging isn't working during InitPostgres either. > The patch addresses this by moving up PostgresMain's > PG_SETMASK(&UnBlockSig); call to before InitPostgres. We might need to > back-patch that bit, though I'm hesitant to fool with such a thing in > back branches. The deadlock can only fail to be detected by someone else if the whole initpostgres thing takes longer than deadlock_timout I think. So it doesn't seem very likely. Not sure how likely problems due to missed SI messages are. > > Thoughts, objections, better ideas? > > regards, tom lane > > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > > -- greg http://mit.edu/~gsstark/resume.pdf -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
