Re: [HACKERS] refactoring comment.c
Robert Haas writes: > On Thu, Aug 19, 2010 at 3:34 PM, Tom Lane wrote: >> Robert Haas writes: >>> Any other kibitzing before I commit this? >> >> Sure ... >> >> [kibitzing] > v4. For my money, you could just have said "Applied with suggested changes". They were pretty darn trivial. 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] bg worker: patch 1 of 6 - permanent process
On Thu, Aug 26, 2010 at 3:40 PM, Markus Wanner wrote: > On 08/26/2010 09:22 PM, Tom Lane wrote: >> >> Not having to have a hard limit on the space for unconsumed messages? > > Ah, I see. However, spilling to disk is unwanted for the current use cases > of imessages. Instead the sender needs to be able to deal with > out-of-(that-specific-part-of-shared)-memory conditions. Shared memory can be paged out, too, if it's not being used enough to keep the OS from deciding to evict it. And I/O to a mmap()'d file or shared memory region can remain in RAM. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- 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] bg worker: patch 1 of 6 - permanent process
On Thu, Aug 26, 2010 at 3:03 PM, Markus Wanner wrote: >> On the more general topic of imessages, I had one other thought that >> might be worth considering. Instead of using shared memory, what >> about using a file that is shared between the sender and receiver? > > What would that buy us? (At the price of more system calls and disk I/O)? > Remember that the current approach (IIRC) uses exactly one syscall to send a > message: kill() to send the (multiplexed) signal. (Except on strange > platforms or setups that don't have a user-space spinlock implementation and > need to use system mutexes). It wouldn't require you to preallocate a big chunk of shared memory without knowing how much of it you'll actually need. For example, suppose we implement parallel query. If the message queues can be allocated on the fly, then you can just say maximum_message_queue_size_per_backend = 16MB and that'll probably be good enough for most installations. On systems where parallel query is not used (e.g. because they only have 1 or 2 processors) then it costs nothing. On systems where parallel query is used extensively (e.g. because they have 32 processors), you'll allocate enough space for the number of backends that actually need message buffers, and not more than that. Furthermore, if parallel query is used at some times (say, for nightly reporting) but not others (say, for daily OLTP queries), the buffers can be deallocated when the helper backends exit (or paged out if they are idle), and that memory can be reclaimed for other use. In addition, it means that maximum_message_queue_size_per_backend (or whatever it's called) can be changed on-the-fly; that is, it can be PGC_SIGHUP rather than PGC_POSTMASTER. Being able to change GUCs without shutting down the postmaster is a *big deal* for people running in 24x7 operations. Even things like wal_level that aren't apt to be changed more than once in a blue moon are a problem (once you go from "not having a standby" to "having a standby", you're unlikely to want to go backwards), and this would likely need more tweaking. You might find that you need more memory for better throughput, or that you need to reclaim memory for other purposes. Especially if it's a hard allocation for any number of backends, rather than something that backends can allocate only as and when they need it. As to efficiency, the process is not much different once the initial setup is completed. Just because you write to a memory-mapped file rather than a shared memory segment doesn't mean that you're necessarily doing disk I/O. On systems that support it, you could also choose to map a named POSIX shm rather than a disk file. Either way, there might be a little more overhead at startup but that doesn't seem so bad; presumably the amount of work that the worker is doing is large compared to the overhead of a few system calls, or you're probably in trouble anyway, since our process startup overhead is pretty substantial already. The only time it seems like the overhead would be annoying is if a process is going to use this system, but only lightly. Doing the extra setup just to send one or two messages might suck. But maybe that just means this isn't the right mechanism for those cases (e.g. the existing XID-wraparound logic should still use signal multiplexing rather than this system). I see the value of this as being primarily for streaming big chunks of data, not so much for sending individual, very short messages. >> On >> the other hand, for processes that only send and receive messages >> occasionally, this might just be overkill (and overhead). You'd be >> just as well off wrapping the access to the file in an LWLock: the >> reader takes the lock, reads the data, marks it read, and releases the >> lock. The writer takes the lock, writes data, and releases the lock. > > The current approach uses plain spinlocks, which are more efficient. Note > that both, appending as well as removing from the queue are writing > operations, from the point of view of the queue. So I don't think LWLocks > buy you anything here, either. I agree that this might not be useful. We don't really have all the message types defined yet, though, so it's hard to say. > I understand the need to limit the amount of data in flight, but I don't > think that sending any type of message should ever block. Messages are > atomic in that regard. Either they are ready to be delivered (in entirety) > or not. Thus the sender needs to hold back the message, if the recipient is > overloaded. (Also note that currently imessages are bound to a maximum size > of around 8 KB). That's functionally equivalent to blocking, isn't it? I think that's just a question of what API you want to expose. > It might be interesting to note that I've just implemented some kind of > streaming mechanism *atop* of imessages for Postgres-R. A data stream gets > fragmented into single messages. As you pointed out, there should be some
Re: [HACKERS] [BUGS] BUG #5305: Postgres service stops when closing Windows session
I still believe this "exit code 128" is related to pgAdmin opened during the clossing session on Remote Desktop. I have a Windows user login wich is not administrator just no privileged user, it cannot start/stop services, just monitoring. With pgAdmin window opened inside my disconected session, as Administrator if I "close" the another disconnected session, Postgres exit with 128 code. Did you reproduce this behavior? Cristian. 2010/8/26 Tom Lane > Robert Haas writes: > > On Tue, Aug 24, 2010 at 9:58 AM, Tom Lane wrote: > >> Given the existence of the deadman switch mechanism (which I hadn't > >> remembered when this thread started), I'm coming around to the idea that > >> we could just treat exit(128) as nonfatal on Windows. If for some > >> reason the child hadn't died instantly at startup, the deadman switch > >> would distinguish that from the case described here. > > > So do you want to code this up? > > Who, me? I don't do Windows --- I'd have no way to test it. > >regards, tom lane >
Re: [HACKERS] About the types of fields in a data structure
Pei He writes: > When I check the types of fields in a data structure, I found most > fields are defined as general types, as List, Node. Then, To know the > content inside the List, I need to track the usage of the fields. > For example, the fromClause in SelectStmt is defined as List. And, the > content in the ListCell is with the type of RangeVar. > Is there other easier way to check the types information, rather than > track through the code? Usually, if a particular List field is predictably a list of just one type of node, the declaration is (or should be) commented to tell you that. Look at struct Query in parsenodes.h for some examples. 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] CopyReadLineText optimization revisited
Heikki Linnakangas writes: > On 26/08/10 22:16, Tom Lane wrote: >> I think this is likely to break apps that have worked for years. I >> can't get excited about doing that in return for an "0-10%" speedup >> that might only materialize on some platforms. If the numbers were >> better, it'd be worth paying that price, but ... > Ok. If we have to, we can keep that, it just requires more programming. > After searching for a \n, we can peek at the previous byte to check if > it's a backslash (and if it is, the one before that to see if it's a > backslash too, and so forth until we find a non-backslash). I seem to recall that this same problem was discussed before, and we concluded that you couldn't reliably parse backwards to figure out whether the newline was backslashed. Although that might have been in the context of data still in client encoding, where you have the extra frammish that a "backslash" could be a non-first byte of a character. Anyway it'd be a good idea to recheck those old discussions if you can find 'em. Another approach that came to me was to parse forwards, and if we find a backslash at the end of the line, then conclude that we had a backslashed newline and slurp in another line's worth of data at that time. I'm not sure how much restructuring would be needed to make that feasible, but it seems worth considering. 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
[HACKERS] About the types of fields in a data structure
Hi, When I check the types of fields in a data structure, I found most fields are defined as general types, as List, Node. Then, To know the content inside the List, I need to track the usage of the fields. For example, the fromClause in SelectStmt is defined as List. And, the content in the ListCell is with the type of RangeVar. Is there other easier way to check the types information, rather than track through the code? Or is there any document about it? Thanks -- Pei -- 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] CopyReadLineText optimization revisited
On 26/08/10 22:16, Tom Lane wrote: Heikki Linnakangas writes: * instead of the byte-at-a-time loop in CopyReadLineText(), use memchr() to find the next NL/CR character. This is where the speedup comes from. That seems like the speedup, if any, would be extremely platform-dependent. What have you tested on? Only on my 32-bit Intel Linux laptop. If anyone out there has more interesting platforms to test on, that would be appreciated. There's a small fly in the ointment: the patch won't recognize backslash followed by a linefeed as an escaped linefeed. I think we should simply drop support for that. I think this is likely to break apps that have worked for years. I can't get excited about doing that in return for an "0-10%" speedup that might only materialize on some platforms. If the numbers were better, it'd be worth paying that price, but ... Ok. If we have to, we can keep that, it just requires more programming. After searching for a \n, we can peek at the previous byte to check if it's a backslash (and if it is, the one before that to see if it's a backslash too, and so forth until we find a non-backslash). -- 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] bg worker: patch 1 of 6 - permanent process
On 08/26/2010 09:22 PM, Tom Lane wrote: Not having to have a hard limit on the space for unconsumed messages? Ah, I see. However, spilling to disk is unwanted for the current use cases of imessages. Instead the sender needs to be able to deal with out-of-(that-specific-part-of-shared)-memory conditions. Please note the coding rule that says that the code should not execute more than a few straight-line instructions while holding a spinlock. If you're copying long messages while holding the lock, I don't think spinlocks are acceptable. Writing the payload data for imessages to shared memory doesn't need any kind of lock. (Because the relevant chunk of shared memory got allocated via wamalloc, which grants the allocator exclusive control over the returned chunk). Only appending and removing (the pointer to the data) to and from the queue requires taking a spinlock. And I think that still qualifies. However, your concern is valid for wamalloc, which is more critical in that regard. Regards Markus -- 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] bg worker: patch 1 of 6 - permanent process
Markus Wanner writes: > On 08/26/2010 02:44 PM, Robert Haas wrote: >> On the more general topic of imessages, I had one other thought that >> might be worth considering. Instead of using shared memory, what >> about using a file that is shared between the sender and receiver? > What would that buy us? Not having to have a hard limit on the space for unconsumed messages? > The current approach uses plain spinlocks, which are more efficient. Please note the coding rule that says that the code should not execute more than a few straight-line instructions while holding a spinlock. If you're copying long messages while holding the lock, I don't think spinlocks are acceptable. 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] CopyReadLineText optimization revisited
Heikki Linnakangas writes: > * we perform possible encoding conversion early, one input block at a > time, rather than after splitting the input into lines. This allows us > to assume in the later stages that the data is in server encoding, > allowing us to search for the '\n' byte without worrying about > multi-byte characters. Seems reasonable, although the need to deal with multibyte characters crossing a block boundary injects some ugliness that wasn't there before. > * instead of the byte-at-a-time loop in CopyReadLineText(), use memchr() > to find the next NL/CR character. This is where the speedup comes from. That seems like the speedup, if any, would be extremely platform-dependent. What have you tested on? > There's a small fly in the ointment: the patch won't recognize backslash > followed by a linefeed as an escaped linefeed. I think we should simply > drop support for that. I think this is likely to break apps that have worked for years. I can't get excited about doing that in return for an "0-10%" speedup that might only materialize on some platforms. If the numbers were better, it'd be worth paying that price, but ... > It's not strictly necessary, but how about dropping support for the old > COPY protocol, and the EOF marker \. while we're at it? It would allow > us to drop some code, making the remaining code simpler, and reduce the > testing effort. Thoughts on that? Again, I think the threshold requirement for breaking compatibility needs to be a lot higher than this. 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] [ADMIN] Unable to drop role
Excerpts from Tom Lane's message of jue ago 26 14:59:43 -0400 2010: > Alvaro Herrera writes: > > Code is here: > > else if (deptype == SHARED_DEPENDENCY_ACL) > > appendStringInfo(descs, _("access to %s"), objdesc); > > in StoreObjectDescription(). > > > Happy to change it to whatever is deemed appropriate. "privileges for %s" > > sounds good; I'll do that unless somebody comes up with a better idea > > which outvotes this one. > > If you're not able to commit this in the next couple of hours, please > let me know and I'll do it. RC1 wraps tonight. Ok, I'll commit it soon, thanks for the notice. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] bg worker: patch 1 of 6 - permanent process
Robert, On 08/26/2010 02:44 PM, Robert Haas wrote: I dunno. It was just a thought. I haven't actually looked at the code to see how much synergy there is. (Sorry, been really busy...) No problem, was just wondering if there's any benefit you had in mind. On the more general topic of imessages, I had one other thought that might be worth considering. Instead of using shared memory, what about using a file that is shared between the sender and receiver? What would that buy us? (At the price of more system calls and disk I/O)? Remember that the current approach (IIRC) uses exactly one syscall to send a message: kill() to send the (multiplexed) signal. (Except on strange platforms or setups that don't have a user-space spinlock implementation and need to use system mutexes). So for example, perhaps each receiver will read messages from a file called pg_messages/%d, where %d is the backend ID. And writers will write into that file. Perhaps both readers and writers mmap() the file, or perhaps there's a way to make it work with just read() and write(). If you actually mmap() the file, you could probably manage it in a fashion pretty similar to what you had in mind for wamalloc, or some other setup that minimizes locking. That would still require proper locking, then. So I'm not seeing the benefit. In particular, ISTM that if we want this to be usable for parallel query, we'll want to be able to have one process streaming data in while another process streams data out, with minimal interference between these two activities. That's well possible with the current approach. About the only limitation is that a receiver can only consume the messages in the order they got into the queue. But pretty much any backend can send messages to any other backend concurrently. (Well, except that I think there currently are bugs in wamalloc). On the other hand, for processes that only send and receive messages occasionally, this might just be overkill (and overhead). You'd be just as well off wrapping the access to the file in an LWLock: the reader takes the lock, reads the data, marks it read, and releases the lock. The writer takes the lock, writes data, and releases the lock. The current approach uses plain spinlocks, which are more efficient. Note that both, appending as well as removing from the queue are writing operations, from the point of view of the queue. So I don't think LWLocks buy you anything here, either. It almost seems to me that there are two different kinds of messages here: control messages and data messages. Control messages are things like "vacuum this database!" or "flush your cache!" or "execute this query and send the results to backend %d!" or "cancel the currently executing query!". They are relatively small (in some cases, fixed-size), relatively low-volume, don't need complex locking, and can generally be processed serially but with high priority. Data messages are streams of tuples, either from a remote database from which we are replicating, or between backends that are executing a parallel query. These messages may be very large and extremely high-volume, are very sensitive to concurrency problems, but are not high-priority. We want to process them as quickly as possible, of course, but the work may get interrupted by control messages. Another point is that it's reasonable, at least in the case of parallel query, for the action of sending a data message to *block*. If one part of the query is too far ahead of the rest of the query, we don't want to queue up results forever, perhaps using CPU or I/O resources that some other backend needs to catch up, exhausting available disk space, etc. I agree that such a thing isn't currently covered. And it might be useful. However, adding two separate queues with different priority would be very simple to do. (Note, however, that there already are the standard unix signals for very simple kinds of control signals. I.e. for aborting a parallel query, you could simply send SIGINT to all background workers involved). I understand the need to limit the amount of data in flight, but I don't think that sending any type of message should ever block. Messages are atomic in that regard. Either they are ready to be delivered (in entirety) or not. Thus the sender needs to hold back the message, if the recipient is overloaded. (Also note that currently imessages are bound to a maximum size of around 8 KB). It might be interesting to note that I've just implemented some kind of streaming mechanism *atop* of imessages for Postgres-R. A data stream gets fragmented into single messages. As you pointed out, there should be some kind of congestion control. However, in my case, that needs to cover the inter-node connection as well, not just imessages. So I think the solution to that problem needs to be found on a higher level. I.e. in the Postgres-R case, I want to limit the *overall* amount of recovery
Re: [HACKERS] [ADMIN] Unable to drop role
Alvaro Herrera writes: > Code is here: > else if (deptype == SHARED_DEPENDENCY_ACL) > appendStringInfo(descs, _("access to %s"), > objdesc); > in StoreObjectDescription(). > Happy to change it to whatever is deemed appropriate. "privileges for %s" > sounds good; I'll do that unless somebody comes up with a better idea > which outvotes this one. If you're not able to commit this in the next couple of hours, please let me know and I'll do it. RC1 wraps tonight. 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] [BUGS] BUG #5305: Postgres service stops when closing Windows session
Robert Haas writes: > On Tue, Aug 24, 2010 at 9:58 AM, Tom Lane wrote: >> Given the existence of the deadman switch mechanism (which I hadn't >> remembered when this thread started), I'm coming around to the idea that >> we could just treat exit(128) as nonfatal on Windows. If for some >> reason the child hadn't died instantly at startup, the deadman switch >> would distinguish that from the case described here. > So do you want to code this up? Who, me? I don't do Windows --- I'd have no way to test it. 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] [BUGS] BUG #5305: Postgres service stops when closing Windows session
On Tue, Aug 24, 2010 at 9:58 AM, Tom Lane wrote: > Bruce Momjian writes: >> Robert Haas wrote: >>> Yeah, that seems very plausible, although exactly how to verify I don't >>> know. > >> And here is confirmation from the Microsoft web site: > >> In some instances, calling GetExitCode() against the failed process >> indicates the following exit code: >> 128L ERROR_WAIT_NO_CHILDREN - There are no child processes to wait for. > > Given the existence of the deadman switch mechanism (which I hadn't > remembered when this thread started), I'm coming around to the idea that > we could just treat exit(128) as nonfatal on Windows. If for some > reason the child hadn't died instantly at startup, the deadman switch > would distinguish that from the case described here. So do you want to code this up? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] CopyReadLineText optimization revisited
I'm reviving the effort I started a while back to make COPY faster: http://archives.postgresql.org/pgsql-patches/2008-02/msg00100.php http://archives.postgresql.org/pgsql-patches/2008-03/msg00015.php The patch I now have is based on using memchr() to search end-of-line. In a nutshell: * we perform possible encoding conversion early, one input block at a time, rather than after splitting the input into lines. This allows us to assume in the later stages that the data is in server encoding, allowing us to search for the '\n' byte without worrying about multi-byte characters. * instead of the byte-at-a-time loop in CopyReadLineText(), use memchr() to find the next NL/CR character. This is where the speedup comes from. Unfortunately we can't do that in the CSV codepath, because newlines can be embedded in quoted, so that's unchanged. These changes seem to give an overall speedup of between 0-10%, depending on the shape of the table. I tested various tables from the TPC-H schema, and a narrow table consisting of just one short text column. I can't think of a case where these changes would be a net loss in performance, and it didn't perform worse on any of the cases I tested either. There's a small fly in the ointment: the patch won't recognize backslash followed by a linefeed as an escaped linefeed. I think we should simply drop support for that. The docs already say: It is strongly recommended that applications generating COPY data convert data newlines and carriage returns to the \n and \r sequences respectively. At present it is possible to represent a data carriage return by a backslash and carriage return, and to represent a data newline by a backslash and newline. However, these representations might not be accepted in future releases. They are also highly vulnerable to corruption if the COPY file is transferred across different machines (for example, from Unix to Windows or vice versa). I vaguely recall that we discussed this some time ago already and agreed that we can drop it if it makes life easier. This patch is in pretty good shape, however it needs to be tested with different exotic input formats. Also, the loop in CopyReadLineText could probaby be cleaned up a bit, some of the uglifications that were done for performance reasons in the old code are no longer necessary, as memchr() is doing the heavy-lifting and the loop only iterates 1-2 times per line in typical cases. It's not strictly necessary, but how about dropping support for the old COPY protocol, and the EOF marker \. while we're at it? It would allow us to drop some code, making the remaining code simpler, and reduce the testing effort. Thoughts on that? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com *** a/src/backend/commands/copy.c --- b/src/backend/commands/copy.c *** *** 143,167 typedef struct CopyStateData /* * Similarly, line_buf holds the whole input line being processed. The ! * input cycle is first to read the whole line into line_buf, convert it ! * to server encoding there, and then extract the individual attribute ! * fields into attribute_buf. line_buf is preserved unmodified so that we ! * can display it in error messages if appropriate. */ StringInfoData line_buf; - bool line_buf_converted; /* converted to server encoding? */ /* ! * Finally, raw_buf holds raw data read from the data source (file or ! * client connection). CopyReadLine parses this data sufficiently to ! * locate line boundaries, then transfers the data to line_buf and ! * converts it. Note: we guarantee that there is a \0 at ! * raw_buf[raw_buf_len]. */ #define RAW_BUF_SIZE 65536 /* we palloc RAW_BUF_SIZE+1 bytes */ char *raw_buf; int raw_buf_index; /* next byte to process */ int raw_buf_len; /* total # of bytes stored */ } CopyStateData; typedef CopyStateData *CopyState; --- 143,175 /* * Similarly, line_buf holds the whole input line being processed. The ! * input cycle is first to convert the input to server encoding in ! * raw_buf, then read the whole line into line_buf, and then extract the ! * individual attribute fields into attribute_buf. line_buf is preserved ! * unmodified so that we can display it in error messages if appropriate. */ StringInfoData line_buf; /* ! * raw_buf holds raw data read from the data source (file or client ! * connection), converted to server encoding if necessary. CopyReadLine ! * parses this data sufficiently to locate line boundaries, then ! * transfers the data to line_buf. Note: we guarantee that there is ! * a \0 at raw_buf[raw_buf_len]. */ #define RAW_BUF_SIZE 65536 /* we palloc RAW_BUF_SIZE+1 bytes */ char *raw_buf; int raw_buf_index; /* next byte to process */ int raw_buf_len; /* total # of bytes stored */ + + /* + * Finally, unconverted_buf holds residual raw data read from d
Re: [HACKERS] Committers info for the git migration - URGENT!
> I wonder whether we shouldn't use the most recent address we have for > the git conversion, though. > Thomas, do you have a preference? See > http://archives.postgresql.org/pgsql-hackers/2010-08/msg01078.php for > context. I'd already replied directly to Alvaro: lockh...@fourpalms.org is fine. Cheers. - Tom
Re: [HACKERS] Committers info for the git migration - URGENT!
Bruce Momjian writes: > Alvaro Herrera wrote: >>> 'thomas' : ('Thomas G. Lockhart', 'lockh...@alumni.caltech.edu'), >> >> Curious about this address -- in his farewell message, Thomas used >> lockh...@fourpalms.org: > Yes, both emails were valid. The fourplams is one he used only toward > the end of his Postgres involvement. I wonder whether we shouldn't use the most recent address we have for the git conversion, though. Thomas, do you have a preference? See http://archives.postgresql.org/pgsql-hackers/2010-08/msg01078.php for context. 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] Committers info for the git migration - URGENT!
Alvaro Herrera wrote: > Excerpts from Magnus Hagander's message of jue ago 26 09:55:34 -0400 2010: > > > 'thomas' : ('Thomas G. Lockhart', 'lockh...@alumni.caltech.edu'), > > Curious about this address -- in his farewell message, Thomas used > lockh...@fourpalms.org: > > http://archives.postgresql.org/pgsql-hackers/2002-11/msg00669.php > > But following that reference, we find a different address: > http://www.fourpalms.org/~lockhart/index.html > Thomas.Lockhart at jpl.nasa.gov Yes, both emails were valid. The fourplams is one he used only toward the end of his Postgres involvement. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Typing Records
On Aug 26, 2010, at 9:05 AM, Tom Lane wrote: > On reflection, I think that the current system design for this is > predicated on the theory that RECORDs really are all the same type, and > the executor had better be prepared to cope with a series of RECORDs > that have different tupdescs, or throw error if it cannot. We can see > that theory at work in record_out() for example. On that theory, the > blame for this crash should be pinned on ExecMakeTableFunctionResult(), > which is assuming that all records returned by the SRF will have the > same typmod. It ought to check that instead of just assuming. > > I like this theory mainly because it leads to a back-patchable fix in > just one place. Trying to get the parser to enforce safety at parse > time was turning into a horrendous mess :-( Sorry for the pain, Tom. David -- 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] Typing Records
I wrote: > The issue seems to be that given a construct like > ARRAY[ > ROW('1.2.2'::semver, '='::text, '1.2.2'::semver), > ROW('1.2.23', '=', '1.2.23') > ] > the parser is satisfied upon finding that all the array elements are > of type RECORD. It doesn't do anything to make sure they are all of > the *same* anonymous record type ... and here they are not. The > second one is just RECORD(UNKNOWN, UNKNOWN, UNKNOWN), which doesn't > even have a compatible representation with the first one. So at runtime > we end up trying to disassemble a tuple containing three UNKNOWN fields > using a tupledesc for the other rowtype. > I think it wouldn't take too much code to defend against this in > transformArrayExpr, but I'm a tad worried about whether there are > similar cases elsewhere. The generic problem is that we suppose that > different values are compatible if they have the same type OID, but > for RECORD types that's really not true. On reflection, I think that the current system design for this is predicated on the theory that RECORDs really are all the same type, and the executor had better be prepared to cope with a series of RECORDs that have different tupdescs, or throw error if it cannot. We can see that theory at work in record_out() for example. On that theory, the blame for this crash should be pinned on ExecMakeTableFunctionResult(), which is assuming that all records returned by the SRF will have the same typmod. It ought to check that instead of just assuming. I like this theory mainly because it leads to a back-patchable fix in just one place. Trying to get the parser to enforce safety at parse time was turning into a horrendous mess :-( 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] Committers info for the git migration - URGENT!
Excerpts from Magnus Hagander's message of jue ago 26 09:55:34 -0400 2010: > 'thomas' : ('Thomas G. Lockhart', 'lockh...@alumni.caltech.edu'), Curious about this address -- in his farewell message, Thomas used lockh...@fourpalms.org: http://archives.postgresql.org/pgsql-hackers/2002-11/msg00669.php But following that reference, we find a different address: http://www.fourpalms.org/~lockhart/index.html Thomas.Lockhart at jpl.nasa.gov -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] Committers info for the git migration - URGENT!
Tom Lane wrote: > Bruce Momjian writes: > > Dan McGuirk is definitely right. I do not remember Julian Assange at > > all. Why do we believe it is Julian Assange? > > There's half a dozen commits from back around 1996 applied by Marc > on behalf of Julian Assange , and then about > half a dozen committed by "julian" without any other ID. The first > of the former is > > 1996-07-18 01:48 scrappy > > * src/: bin/monitor/monitor.c, bin/psql/psql.c, > interfaces/libpq/fe-exec.c, interfaces/libpq/libpq-fe.h: libpq and > psql.c have been modified to do various things they didn't do > before (plus some optimisations/bug fixes et al). I've included a > small demo transcript below. Note that all of of the display > functionality/intelligence you see here, can be had merely by > calling the new LIBPQ PQprint() routine with the appropriate > arguments/options, including the HTML3 output guff. > > submitted by: Julian Assange > > and the last of the latter is > > 1996-08-20 20:22 julian > > * src/bin/psql/psql.c: command line flag for expanded display '-x' > had logic reversed > > So far as I can see, he only contributed to psql not the backend. > > Oh wait, here is one where he identifies himself as "proff": > > 1996-07-25 02:46 julian > > * src/bin/psql/psql.c: Large re-write/enhancement. In pg-101 Jolly > only included a smaller part of my (proff) patch. This is the rest > of it, with a few, mainly aesthetic changes. I've removed a lot of > redundency from the original code, added support for the new > PQprint() routines in libpq, expanded tables, and a few generally > nifty ways of massaging data in and out of the backend. Still needs > some good stress testing. > > So this committer is him, not some other Julian. Yep, I must have forgotten about him. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Committers info for the git migration - URGENT!
On Thu, Aug 26, 2010 at 15:53, Tom Lane wrote: > Magnus Hagander writes: >> On Thu, Aug 26, 2010 at 00:49, Tom Lane wrote: >>> BTW, I noticed that this list omits several old committers: > >> Added to the list used. > > Just for the archives, would you post the current list? I lost track of > who asked for their email entries to be changed. Sure! 'adunstan' : ('Andrew Dunstan', 'and...@dunslane.net'), 'alvherre' : ('Alvaro Herrera', 'alvhe...@alvh.no-ip.org'), 'barry' : ('Barry Lind', 'ba...@xythos.com'), 'bryanh' : ('Bryan Henderson', 'bry...@giraffe.netgate.net'), 'byronn' : ('Byron Nikolaidis', 'byr...@insightdist.com'), 'darcy' : ('D\'Arcy J.M. Cain', 'da...@druid.net'), 'davec' : ('Dave Cramer', 'da...@fastcrypt.com'), 'dennis' : ('Dennis Bjorklund', 'd...@zigo.dhs.org'), 'heikki' : ('Heikki Linnakangas', 'heikki.linnakan...@iki.fi'), 'inoue' : ('Hiroshi Inoue', 'in...@tpf.co.jp'), 'ishii' : ('Tatsuo Ishii', 'is...@postgresql.org'), 'itagaki' : ('Itagaki Takahiro', 'itagaki.takah...@gmail.com'), 'joe' : ('Joe Conway', 'm...@joeconway.com'), 'julian' : ('Julian Assange', 'pr...@suburbia.net'), 'jurka' : ('Kris Jurka', 'bo...@ejurka.com'), 'mcguirk' : ('Dan McGuirk', 'mcgu...@indirect.com'), 'mergl' : ('Edmund Mergl', 'e.me...@bawue.de'), 'meskes' : ('Michael Meskes', 'mes...@postgresql.org'), 'mha' : ('Magnus Hagander', 'mag...@hagander.net'), 'momjian' : ('Bruce Momjian', 'br...@momjian.us'), 'neilc' : ('Neil Conway', 'ne...@samurai.com'), 'peter' : ('Peter Mount', 'pe...@retep.org.uk'), 'petere' : ('Peter Eisentraut', 'pete...@gmx.net'), 'pgsql' : ('PostgreSQL Daemon', 'webmas...@postgresql.org'), 'pjw' : ('Philip Warner', 'p...@rhyme.com.au'), 'rhaas' : ('Robert Haas', 'rh...@postgresql.org'), 'scrappy' : ('Marc G. Fournier', 'scra...@hub.org'), 'sriggs' : ('Simon Riggs', 'si...@2ndquadrant.com'), 'stark' : ('Greg Stark', 'st...@mit.edu'), 'teodor' : ('Teodor Sigaev', 'teo...@sigaev.ru'), 'tgl' : ('Tom Lane', 't...@sss.pgh.pa.us'), 'thomas' : ('Thomas G. Lockhart', 'lockh...@alumni.caltech.edu'), 'vadim' : ('Vadim B. Mikheev', 'vadi...@yahoo.com'), 'vev' : ('Vince Vielhaber', 'v...@michvhf.com'), 'wieck' : ('Jan Wieck', 'janwi...@yahoo.com'), -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] Committers info for the git migration - URGENT!
Magnus Hagander writes: > On Thu, Aug 26, 2010 at 00:49, Tom Lane wrote: >> BTW, I noticed that this list omits several old committers: > Added to the list used. Just for the archives, would you post the current list? I lost track of who asked for their email entries to be changed. 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] Committers info for the git migration - URGENT!
On Thu, 26 Aug 2010, Bruce Momjian wrote: Magnus Hagander wrote: On Thu, Aug 26, 2010 at 00:49, Tom Lane wrote: Magnus Hagander writes: The current mapping used is the same one as on git.postgresql.org (see attached file). BTW, I noticed that this list omits several old committers: ?162 bryanh ?20 byronn ? 6 julian ? 1 mcguirk (the numbers are the number of commits I find in cvs2cl for each name). I am pretty sure of the first two: Bryan Henderson Byron Nikolaidis and I think the others are Julian Assange Dan McGuirk though they're before my time. Added to the list used. Bruce, can you confirm the last two? I assume you were around back then? ;) Or maybe Marc? Dan McGuirk is definitely right. I do not remember Julian Assange at all. Why do we believe it is Julian Assange? That name definitely rings a bell for me ... Marc G. FournierHub.Org Hosting Solutions S.A. scra...@hub.org http://www.hub.org Yahoo:yscrappySkype: hub.orgICQ:7615664MSN:scra...@hub.org -- 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] Committers info for the git migration - URGENT!
Bruce Momjian writes: > Dan McGuirk is definitely right. I do not remember Julian Assange at > all. Why do we believe it is Julian Assange? There's half a dozen commits from back around 1996 applied by Marc on behalf of Julian Assange , and then about half a dozen committed by "julian" without any other ID. The first of the former is 1996-07-18 01:48 scrappy * src/: bin/monitor/monitor.c, bin/psql/psql.c, interfaces/libpq/fe-exec.c, interfaces/libpq/libpq-fe.h: libpq and psql.c have been modified to do various things they didn't do before (plus some optimisations/bug fixes et al). I've included a small demo transcript below. Note that all of of the display functionality/intelligence you see here, can be had merely by calling the new LIBPQ PQprint() routine with the appropriate arguments/options, including the HTML3 output guff. submitted by: Julian Assange and the last of the latter is 1996-08-20 20:22 julian * src/bin/psql/psql.c: command line flag for expanded display '-x' had logic reversed So far as I can see, he only contributed to psql not the backend. Oh wait, here is one where he identifies himself as "proff": 1996-07-25 02:46 julian * src/bin/psql/psql.c: Large re-write/enhancement. In pg-101 Jolly only included a smaller part of my (proff) patch. This is the rest of it, with a few, mainly aesthetic changes. I've removed a lot of redundency from the original code, added support for the new PQprint() routines in libpq, expanded tables, and a few generally nifty ways of massaging data in and out of the backend. Still needs some good stress testing. So this committer is him, not some other Julian. 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] Committers info for the git migration - URGENT!
Magnus Hagander wrote: > On Thu, Aug 26, 2010 at 00:49, Tom Lane wrote: > > Magnus Hagander writes: > >> The current mapping used is the same one as on git.postgresql.org (see > >> attached file). > > > > BTW, I noticed that this list omits several old committers: > > > > ?162 bryanh > > ?20 byronn > > ? 6 julian > > ? 1 mcguirk > > > > (the numbers are the number of commits I find in cvs2cl for each name). > > > > I am pretty sure of the first two: > > > > Bryan Henderson > > Byron Nikolaidis > > > > and I think the others are > > > > Julian Assange > > Dan McGuirk > > > > though they're before my time. > > Added to the list used. > > Bruce, can you confirm the last two? I assume you were around back > then? ;) Or maybe Marc? Dan McGuirk is definitely right. I do not remember Julian Assange at all. Why do we believe it is Julian Assange? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Performance Farm Release
Right, sorry about that, one of those stupid things I did on a lack of sleep. The github is now here: http://github.com/slux/PostgreSQL-Performance-Farm -Original Message- From: Heikki Linnakangas [mailto:heikki.linnakan...@enterprisedb.com] Sent: Wednesday, August 25, 2010 2:31 PM To: Luxenberg, Scott I. Cc: PostgreSQL-development Subject: Re: [HACKERS] Performance Farm Release (resending as I also accidentally CC'd pgsql-hackers-owner, not the list) On 25/08/10 02:25, Luxenberg, Scott I. wrote: > This is just my email to notify you all that the project I've been > working on with Stephen, the PostgreSQL Performance Farm, has been > released. As of now, it only supports 9.0, due to the use of workers. > More details can be found in the readme. The Git repository is located > here: http://github.com/slux/Postgre-Performance-Farm Umm, how about renaming it to "PostgreSQL-Performance-Farm" before the name gets too engrained everywhere and impossible to change anymore? The project is not called "Postgre".. -- 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] bg worker: patch 1 of 6 - permanent process
On Thu, Aug 26, 2010 at 6:07 AM, Markus Wanner wrote: > What'd be the benefits of having separate coordinator processes? They'd be > doing pretty much the same: coordinate background processes. (And yes, I > clearly consider autovacuum to be just one kind of background process). I dunno. It was just a thought. I haven't actually looked at the code to see how much synergy there is. (Sorry, been really busy...) >> I agree with this criticism, but the other thing that strikes me as a >> nonstarter is having the postmaster participate in the imessages >> framework. > > This is simply not the case (anymore). (And one of the reasons a separate > coordinator process is required, instead of letting the postmaster do this > kind of coordination). Oh, OK. I see now that I misinterpreted what you wrote. On the more general topic of imessages, I had one other thought that might be worth considering. Instead of using shared memory, what about using a file that is shared between the sender and receiver? So for example, perhaps each receiver will read messages from a file called pg_messages/%d, where %d is the backend ID. And writers will write into that file. Perhaps both readers and writers mmap() the file, or perhaps there's a way to make it work with just read() and write(). If you actually mmap() the file, you could probably manage it in a fashion pretty similar to what you had in mind for wamalloc, or some other setup that minimizes locking. In particular, ISTM that if we want this to be usable for parallel query, we'll want to be able to have one process streaming data in while another process streams data out, with minimal interference between these two activities. On the other hand, for processes that only send and receive messages occasionally, this might just be overkill (and overhead). You'd be just as well off wrapping the access to the file in an LWLock: the reader takes the lock, reads the data, marks it read, and releases the lock. The writer takes the lock, writes data, and releases the lock. It almost seems to me that there are two different kinds of messages here: control messages and data messages. Control messages are things like "vacuum this database!" or "flush your cache!" or "execute this query and send the results to backend %d!" or "cancel the currently executing query!". They are relatively small (in some cases, fixed-size), relatively low-volume, don't need complex locking, and can generally be processed serially but with high priority. Data messages are streams of tuples, either from a remote database from which we are replicating, or between backends that are executing a parallel query. These messages may be very large and extremely high-volume, are very sensitive to concurrency problems, but are not high-priority. We want to process them as quickly as possible, of course, but the work may get interrupted by control messages. Another point is that it's reasonable, at least in the case of parallel query, for the action of sending a data message to *block*. If one part of the query is too far ahead of the rest of the query, we don't want to queue up results forever, perhaps using CPU or I/O resources that some other backend needs to catch up, exhausting available disk space, etc. Instead, at some point, we just block and wait for the queue to drain. I suppose there's no avoiding the possibility that sending a control message might also block, but certainly we wouldn't like a control message to block because the relevant queue is full of data messages. So I kind of wonder whether we ought to have two separate systems, one for data and one for control, with someone different characteristics. I notice that one of your bg worker patches is for OOO-messages. I apologize again for not having read through it, but how much does that resemble separating the control and data channels? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- 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] Committers info for the git migration - URGENT!
On Thu, Aug 26, 2010 at 05:45, Tom Lane wrote: > "A.M." writes: >> On Aug 25, 2010, at 6:49 PM, Tom Lane wrote: >>> BTW, I noticed that this list omits several old committers: >>> Julian Assange > >> That is _the_ Julian Assange who is in the news now. Very cool! > > Yowza ... *that* Julian Assange? Cool, but maybe we shouldn't > advertise the connection ;-) Well, that's political, and we've done a pretty good job of keeping the project free of politics so far :-) Plus, we don't generally announce our committers... So we should just stick to our existing policies ;) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] Committers info for the git migration - URGENT!
On Thu, Aug 26, 2010 at 00:49, Tom Lane wrote: > Magnus Hagander writes: >> The current mapping used is the same one as on git.postgresql.org (see >> attached file). > > BTW, I noticed that this list omits several old committers: > > 162 bryanh > 20 byronn > 6 julian > 1 mcguirk > > (the numbers are the number of commits I find in cvs2cl for each name). > > I am pretty sure of the first two: > > Bryan Henderson > Byron Nikolaidis > > and I think the others are > > Julian Assange > Dan McGuirk > > though they're before my time. Added to the list used. Bruce, can you confirm the last two? I assume you were around back then? ;) Or maybe Marc? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] bg worker: patch 1 of 6 - permanent process
Itagaki-san, On 08/26/2010 01:02 PM, Itagaki Takahiro wrote: OK, I see why you proposed coordinator hook (yeah, I call it hook :) rather than adding user-defined processes. I see. If you call that a hook, I'm definitely not a hook-hater ;-) at least not according to your definition. However, we have autovacuum worker processes in addition to normal backend processes. Does it show a fact that there are some jobs we cannot run in normal backends? Hm.. understood. You can use VACUUM from a cron job. And that's the problem autovacuum solves. So in a way, that's just a convenience feature. You want the same for general purpose user defined background processing, right? For example, normal backends cannot do anything in idle time, so a time-based polling job is difficult in backends. It might be ok to fork processes for each interval when the polling interval is long, but it is not effective for short interval cases. I'd like to use such kind of process as an additional stats collector. Did you follow the discussion I had with Dimitri, who was trying something similar, IIRC. See the bg worker - overview thread. There might be some interesting bits thinking into that direction. Regards Markus -- 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] bg worker: patch 1 of 6 - permanent process
On Thu, Aug 26, 2010 at 7:42 PM, Markus Wanner wrote: >> Markus, do you need B? Or A + standard backend processes are enough? > > No, I certainly don't need B. OK, I see why you proposed coordinator hook (yeah, I call it hook :) rather than adding user-defined processes. > Why not just use an ordinary backend to do "user defined background > processing"? It covers all of the API stability and the security issues I've > raised. However, we have autovacuum worker processes in addition to normal backend processes. Does it show a fact that there are some jobs we cannot run in normal backends? For example, normal backends cannot do anything in idle time, so a time-based polling job is difficult in backends. It might be ok to fork processes for each interval when the polling interval is long, but it is not effective for short interval cases. I'd like to use such kind of process as an additional stats collector. -- Itagaki Takahiro -- 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] bg worker: patch 1 of 6 - permanent process
On 08/26/2010 05:01 AM, Itagaki Takahiro wrote: Markus, do you need B? Or A + standard backend processes are enough? If you need B eventually, starting with B might be better. No, I certainly don't need B. Why not just use an ordinary backend to do "user defined background processing"? It covers all of the API stability and the security issues I've raised. Regards Markus Wanner -- 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] gSoC add MERGE command new patch -- merge_v104
On 2010-08-25 12:44 PM +0300, Heikki Linnakangas wrote: On 25/08/10 12:41, Andres Freund wrote: But randomly loosing tuples will make much more people unhappy. At a much more problematic point of time (in production). Hmm, how would you lose tuples? I think what Andres means is: T1 starts a MERGE. INSERT fails because the tuple already exists, but then another transaction, T2, DELETEs that tuple. T1 tries to UPDATE it, but fails because it doesn't exist anymore. Not T1 should go back and INSERT the tuple, but that isn't what happens with this patch, is it? Regards, Marko Tiikkaja -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)
On 24/08/10 02:44, Tom Lane wrote: Heikki Linnakangas writes: [ "latch" proposal ] This seems reasonably clean as far as signal conditions generated internally to Postgres go, but I remain unclear on how it helps for response to actual signals. You can can set the latch in the signal handler. Here's a first attempt at implementing that. To demonstrate how it works, I modified walsender to use the new latch facility, also to respond quickly to SIGHUP and SIGTERM. There's two kinds of latches, local and global. Local latches can only be set from the same process - allowing you to replace pg_usleep() with something that is always interruptible by signals (by setting the latch in the signal handler). The global latches work the same, and indeed the implementation is the same, but the latch resides in shared memory, and can be set by any process attached to shared memory. On Unix, when you set a latch waited for by another process, the setter sends SIGUSR1 to the waiting process, and the signal handler sends the byte to the self-pipe to wake up the select(). On Windows, we use WaitEvent to wait on a latch, and SetEvent to wake up. The difference between global and local latches is that for global latches, the Event object needs to be created upfront at postmaster startup so that its inherited to all child processes, and stored in shared memory. A local Event object can be created only in the process that needs it. I put the code in src/backend/storage/ipc/latch.c now, but it probably ought to go in src/backend/portability instead, with a separate win32_latch.c file for Windows. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 615a7fa..094d0c9 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -55,6 +55,7 @@ #include "miscadmin.h" #include "pg_trace.h" #include "pgstat.h" +#include "replication/walsender.h" #include "storage/fd.h" #include "storage/procarray.h" #include "storage/sinvaladt.h" @@ -1025,6 +1026,13 @@ EndPrepare(GlobalTransaction gxact) /* If we crash now, we have prepared: WAL replay will fix things */ + /* + * Wake up all walsenders to send WAL up to the PREPARE record + * immediately if replication is enabled + */ + if (max_wal_senders > 0) + WalSndWakeup(); + /* write correct CRC and close file */ if ((write(fd, &statefile_crc, sizeof(pg_crc32))) != sizeof(pg_crc32)) { @@ -2005,6 +2013,13 @@ RecordTransactionCommitPrepared(TransactionId xid, /* Flush XLOG to disk */ XLogFlush(recptr); + /* + * Wake up all walsenders to send WAL up to the COMMIT PREPARED record + * immediately if replication is enabled + */ + if (max_wal_senders > 0) + WalSndWakeup(); + /* Mark the transaction committed in pg_clog */ TransactionIdCommitTree(xid, nchildren, children); @@ -2078,6 +2093,13 @@ RecordTransactionAbortPrepared(TransactionId xid, XLogFlush(recptr); /* + * Wake up all walsenders to send WAL up to the ABORT PREPARED record + * immediately if replication is enabled + */ + if (max_wal_senders > 0) + WalSndWakeup(); + + /* * Mark the transaction aborted in clog. This is not absolutely necessary * but we may as well do it while we are here. */ diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 6bcc55c..942d5c2 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -36,6 +36,7 @@ #include "libpq/be-fsstubs.h" #include "miscadmin.h" #include "pgstat.h" +#include "replication/walsender.h" #include "storage/bufmgr.h" #include "storage/fd.h" #include "storage/lmgr.h" @@ -1068,6 +1069,13 @@ RecordTransactionCommit(void) XLogFlush(XactLastRecEnd); /* + * Wake up all walsenders to send WAL up to the COMMIT record + * immediately if replication is enabled + */ + if (max_wal_senders > 0) + WalSndWakeup(); + + /* * Now we may update the CLOG, if we wrote a COMMIT record above */ if (markXidCommitted) diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index 53c2581..9f5d1af 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -66,9 +66,6 @@ bool am_walsender = false; /* Am I a walsender process ? */ int max_wal_senders = 0; /* the maximum number of concurrent walsenders */ int WalSndDelay = 200; /* max sleep time between some actions */ -#define NAPTIME_PER_CYCLE 10L /* max sleep time between cycles - * (100ms) */ - /* * These variables are used similarly to openLogFile/Id/Seg/Off, * but for walsender to read the XLOG. @@ -93,6 +90,7 @@ static volatile sig_atomic_t ready_to_stop = false; static void WalSndSigHupHandler(SIGNAL_ARGS); static void WalSndShutdownHandler(SIGNAL_ARGS); static void WalSndQuickDieHandler(SIGNAL_ARGS); +static void WalSndXLogSendHan
Re: [HACKERS] bg worker: patch 1 of 6 - permanent process
Itagaki-san, thanks for reviewing this. On 08/26/2010 03:39 AM, Itagaki Takahiro wrote: Other changes in the patch doesn't seem be always needed for the purpose. In other words, the patch is not minimal. Hm.. yeah, maybe the separation between step1 and step2 is a bit arbitrary. I'll look into it. The original purpose could be done without IMessage. Agreed, that's the one exception. I've mentioned why that is and I don't currently feel like coding an unneeded variant which doesn't use imessages. Also, min/max_spare_background_workers are not used in the patch at all. You are right, it only starts to get used in step2, so the addition should probably move there, right. (BTW, min/max_spare_background_*helpers* in postgresql.conf.sample is maybe typo.) Uh, correct, thank you for pointing this out. (I've originally named these helper processes before. Merging with autovacuum, it made more sense to name them background *workers*) The most questionable point for me is why you didn't add any hook functions in the coordinator process. Because I'm a hook-hater. ;-) No, seriously: I don't see what problem hooks could have solved. I'm coding in C and extending the Postgres code. Deciding for hooks and an API to use them requires good knowledge of where exactly you want to hook and what API you want to provide. Then that API needs to remain stable for an extended time. I don't think any part of the bg worker infrastructure currently is anywhere close to that. With the patch, you can extend the coordinator protocols with IMessage, but it requires patches to core at handle_imessage(). If you want fully-extensible workers, we should provide a method to develop worker codes in an external plugin. It's originally intended as an internal infrastructure. Offering its capabilities to the outside would requires stabilization, security control and working out an API. All of which is certainly not something I intend to do. Is it possible to develop your own codes in the plugin? If possible, you can use IMessage as a private protocol freely in the plugin. Am I missing something? Well, what problem(s) are you trying to solve with such a thing? I've no idea what direction you are aiming at, sorry. However, it's certainly different or extending bg worker, so it would need to be a separate patch, IMO. Regards Markus Wanner -- 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] gSoC add MERGE command new patch -- merge_v104
On Tue, 2010-08-24 at 18:23 -0700, David Fetter wrote: > On Wed, Aug 25, 2010 at 08:11:18AM +0800, Boxuan Zhai wrote: > > On Wed, Aug 25, 2010 at 4:56 AM, Andres Freund wrote: > > > > The concurrency issues are not involved. I don't know much about > > this part. I think we need more discussion on it. > > I seem to recall Simon had volunteered some of 2ndQuadrant's time on > this. :) I have offered to work on the concurrency issues, though that will be after the main body of work is committed to avoid wasting effort. That seems increasingly likely to happen in next release now, given state of patch and what looks like a very short dev window for 9.1. This is one reason why I'm in favour of bringing something to commit as early as possible, so the additional aspects can be added later. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and Services -- 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] bg worker: patch 1 of 6 - permanent process
Hi, thanks for your feedback on this, it sort of got lost below the discussion about the dynamic shared memory stuff, IMO. On 08/26/2010 04:39 AM, Robert Haas wrote: It's not clear to me whether it's better to have a single coordinator process that handles both autovacuum and other things, or whether it's better to have two separate processes. It has been proposed by Alvaro and/or Tom (IIRC) to reduce code duplication. Compared to the former approach, it certainly seems cleaner that way and it has helped reduce duplicate code a lot. I'm envisioning such a coordinator process to handle coordination of other background processes as well, for example for distributed and/or parallel querying. Having just only one process reduces the amount of interaction required between coordinators (i.e. autovacuum shouldn't ever start on databases for which replication didn't start, yet, as the autovacuum worker would be unable to connect to the database at that stage). It also reduces the amount of extra processes required, and thus I think also general complexity. What'd be the benefits of having separate coordinator processes? They'd be doing pretty much the same: coordinate background processes. (And yes, I clearly consider autovacuum to be just one kind of background process). I agree with this criticism, but the other thing that strikes me as a nonstarter is having the postmaster participate in the imessages framework. This is simply not the case (anymore). (And one of the reasons a separate coordinator process is required, instead of letting the postmaster do this kind of coordination). Our general rule is that the postmaster must avoid touching shared memory; else a backend that scribbles on shared memory might take out the postmaster, leading to a failure of the crash-and-restart logic. That rule is well understood and followed by the bg worker infrastructure patches. If you find code for which that isn't true, please point at it. The crash-and-restart logic should work just as it did with the autovacuum launcher. Regards Markus -- 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] EXPLAIN doesn't show the actual function expression for FunctionScan
Tom Lane writes: > Now you might suggest that the function itself is redundant with the > information given in the FunctionScan node line and so we need only > show the argument list. Unfortunately there are cases where this fails; > in particular, the named function could have been "inlined" by the > planner, meaning that the actual expression could be just about anything > at all. So I think that trying to be cute is a bad idea and we should > just print the nodetree as-is. Oh. +1 then. Regards, -- dim -- 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] patch: Add JSON datatype to PostgreSQL (GSoC, WIP)
Hitoshi Harada writes: > It depends on use cases, but in my mind plain text will do for us. If > we have JavaScript engine in PostgreSQL like pl/v8 and it handles > on-disk format as-is, then we should choose the kind of format, but in > either text or binary format way it is hopeless to have such > compelling environment in the short future. Well, for javascript support, there's another nice thing happening: - plscheme is built on GNU Guile - next version of GNU Guile supports javascript too http://plscheme.projects.postgresql.org/ http://wingolog.org/archives/2009/02/22/ecmascript-for-guile So my current guess at which javascript engine we'd get first would be plscheme. Now I don't know what implication that would have on the binary storage format of javascript or json documents. Regards, -- dim -- 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] Deadlock bug
I thought it would be interesting to see how other databases handle this peculiar deadlock situation. I didn't have access to any Oracle or Sybase databases, but for what it's worth I've tested MySQL. Results: 1. Process 1 successfully made its update and managed to commit. 2. Process 1 second update did not went straight through, but had to wait for process 2 to attempt to commit. 3. Process 2 did not manage to commit, all its updates were discarded. Demo of the test: http://screencast.com/t/ZGJmMTcxN /Joel 2010/8/25 Robert Haas : > On Wed, Aug 25, 2010 at 10:02 AM, Simon Riggs wrote: >> On Wed, 2010-08-25 at 15:51 +0200, Markus Wanner wrote: >>> Simon, >>> >>> On 08/25/2010 11:53 AM, Simon Riggs wrote: >>> > ..we want to ensure that the PK value.. >>> >>> ..or any other possibly referenced attributes? >> >> Don't think that's relevant. >> >> "referenced" meaning "by an RI constraint", which only ever refers to >> PKs in other tables. > > That doesn't appear to be correct: > > rhaas=# create table p (a integer primary key, b integer not null, > unique (b)); NOTICE: CREATE TABLE / PRIMARY KEY will create implicit > index "p_pkey" for table "p" > NOTICE: CREATE TABLE / UNIQUE will create implicit index "p_b_key" > for table "p" > CREATE TABLE > rhaas=# create table r (b integer not null references p (b)); > CREATE TABLE > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise Postgres Company > -- Best regards, Joel Jacobson Glue Finance E: j...@gluefinance.com T: +46 70 360 38 01 Postal address: Glue Finance AB Box 549 114 11 Stockholm Sweden Visiting address: Glue Finance AB Birger Jarlsgatan 14 114 34 Stockholm Sweden -- 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] Deadlock bug
Hi, On 08/25/2010 10:35 PM, Simon Riggs wrote: If the row is "key share" locked (as opposed to "tuple share" locks we already have), then an UPDATE would only work if it was a non-HOT UPDATE. I think you meant it the other way around: an UPDATE on a "key share" locked tuple only works if it *was* a HOT UPDATE (which doesn't change any indexed attribute, so RI is guaranteed to be okay). There have been at least three variants proposed for such a "key share" lock, for what kind of updates they should block: a) block UPDATEs that change any indexed attributes *and* block UPDATEs on tuples that don't fit into the same page again (i.e. 100% like the HOT conditions). b) block UPDATEs that change any indexed attributes (i.e. any kind of index on them), but allow non-HOT UPDATEs which change only non-indexed attributes c) block UPDATEs that change any key columns (i.e. only attributes on which there is a UNIQUE constraint), but allow non-HOT UPDATEs which change only non-key attributes. AFAICT b) and c) are sufficient for solving the OPs deadlock bug (i.e. UPDATEs to only non-index or even only non-key attributes). However, a) does not. Instead it would still deadlock for seemingly random occasions. The lock to solve the OPs problem cannot match the HOT conditions 100%. I currently don't see any reason for making it as HOT-like as possible, so I'm advocating variant c). If the reason is simply ease of implementation, I'd be fine with implementing b) first, but the definition of the LOCK type should then leave the way open to the less restrictive variant c). Regards Markus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers