Re: [HACKERS] Planning counters in pg_stat_statements
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Thomas Munro > I have often wanted $SUBJECT and was happy to find that Fujii-san had posted > a patch five years ago[1]. The reception then seemed positive. > So here is a refurbished and (hopefully) improved version of his patch with > a new column for the replan count. Thoughts? That's a timely proposal. I sometimes faced performance problems where the time pg_stat_statements shows is much shorter than the application perceives. The latest experience was that the execution time of a transaction, which consists of dozens of DMLs and COMMIT, was about 200ms from the application's perspective, while pg_stat_statements showed only about 10ms in total. The network should not be the cause because the application ran on the same host as the database server. I wanted to know how long the parsing and planning time was. BTW, the current pg_stat_statement shows unexpected time for COMMIT. I expect it to include the whole COMMIT processing, including the long WAL flush and sync rep wait. However, it only shows the time for the transaction state change in memory. Regards Takayuki Tsunakawa -- 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] Statement-level rollback
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Simon Riggs > A backend-based solution is required for PL procedures and functions. > > We could put this as an option into PL/pgSQL, but it seems like it is > a function of the transaction manager rather than the driver. Exactly. Thanks. Regards Takayuki Tsunakawa -- 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] Statement-level rollback
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Vladimir > Sitnikov > Tsunakawa> PgJDBC has supported the feature with autosave parameter only > Tsunakawa> recently > > PgJDBC has the implementation for more than a year (REL9.4.1210, 2016-09-07, > see https://github.com/pgjdbc/pgjdbc/pull/477 ) And I heard from someone in PgJDBC community that the autosave parameter was not documented in the manual for a while, which I confirmed. So the statement-level rollback is newer to users, isn't it? > The performance overhead for "SELECT" statement (no columns, just select) > statement over localhost is 36±4 us vs 38±3 us (savepoint is pipelined along > with user-provided query). That is network overhead is close to negligible. That's good news, because it also means that the overhead of creating a savepoint is negligible. > As far as I understand, the main problem with savepoints is they would > consume memory even in case the same savepoint is reassigned again and again. > In other words, "savepoint; insert;savepoint; insert;savepoint; > insert;savepoint; insert;savepoint; insert;" would allocate xids and might > blow up backend's memory. > I see no way driver can workaround that, so it would be great if backend > could release memory or provide a way to do so. Doesn't PgJDBC execute RELEASE after each SQL statement? That said, even with RELEASE, the server memory bloat is not solved. The current server implementation allocates a memory chunk of 8KB called CurTranContext for each subtransaction, and retains them until the end of top-level transaction. That's another (separate) issue to address. Regards Takayuki Tsunakawa -- 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] Statement-level rollback
From: Craig Ringer [mailto:cr...@2ndquadrant.com] > The example often cited is some variant of > > BEGIN; > CREATTE TABLE t2 AS SELECT * FROM t1; > DROP TABLE t1; > ALTER TABLE t2 RENAME TO t1; > COMMIT; > > Right now, we do the right thing here. With default statement level rollback, > you just dropped t1 and all your data. oops. That's a horrible example. So I think the default behavior should be what it is now for existing PostgreSQL users. > On a related note, psql's -v ON_ERROR_STOP=1 is horrible and hard to discover > UI, and one of the top FAQs on Stack Overflow is some variant of "I'm getting > random and incomprehensible errors restoring a dump, wtf?". So I'd really > love to make it the default, but we'd face similar issues where a SQL script > that's currently correct instead produces dangerously wrong results with > ON_ERROR_STOP=1 . Yes. And although unrelated, psql's FETCH_SIZE is also often invisible to users. They report out-of-memory trouble when they do SELECT on a large table with psql. > What about if we add protocol-level savepoint support? Two new messages: > > BeginUnnamedSavepoint > > and > > EndUnnamedSavepoint > > where the latter does a rollback-to-last-unnamed-savepoint if the txn state > is bad, or a release-last-unnamed-savepoint if the txn state is ok. That > means the driver doesn't have to wait for the result of the statement. It > knows the conn state and query outcome from our prior messages, and knows > that as a result of this message any failed state has been rolled back. > > This would, with appropriate libpq support, give people who want statement > level error handling pretty much what they want. And we could expose it > in psql too. No GUCs needed, no fun surprises for apps. psqlODBC could adopt > it to replace its current slow and super-log-spammy statement rollback > model. > > Downside is that it needs support in each client driver. Yes, I believe we should avoid the downside. It's tough to develop and maintain a client driver, so we should minimize the burdon with server-side support. Regards Takayuki Tsunakawa -- 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] Statement-level rollback
From: Peter Eisentraut [mailto:peter.eisentr...@2ndquadrant.com] > The difference is how error recovery works. So this will necessarily be > tied to how the client code or other surrounding code is structured or what > the driver or framework is doing in the background to manage transactions. > It would also be bad if client code was not prepared for this new behavior, > reported the transaction as complete while some commands in the middle were > omitted. > > Drivers can already achieve this behavior and do do that by issuing savepoint > commands internally. The point raised in this thread was that that creates > too much network overhead, so a backend-based solution would be preferable. > We haven't seen any numbers or other evidence to quantify that claim, so > maybe it's worth looking into that some more. > > In principle, a backend-based solution that drivers just have to opt into > would save a lot of duplication. But the drivers that care or require it > according to their standards presumably already implement this behavior > in some other way, so it comes back to whether there is a performance or > other efficiency gain here. > > Another argument was that other SQL implementations have this behavior. > This appears to be the case. But as far as I can tell, it is also tied > to their particular interfaces and the structure and flow control they > provide. So a client-side solution like psql already provides or something > in the various drivers would work just fine here. > > So my summary for the moment is that a GUC or similar run-time setting might > be fine, with appropriate explanation and warnings. But it's not clear > whether it's worth it given the existing alternatives. I can think of four reasons why the server-side support is necessary or desirable. First, the server log could be filled with SAVEPOINT and RELEASE lines when you need to investigate performance or audit activity. Second, the ease of use for those who migrate from other DBMSs. With the server-side support, only the DBA needs to be aware of the configuration in postgresql.conf. Other people don't need to be aware of the client-side parameter when they deploy applications. Third, lack of server-side support causes trouble to driver developers. In a recent discussion with the psqlODBC committer, he had some trouble improving or fixing the statement-rollback support. Npgsql doesn't have the statement-rollback yet. PgJDBC has supported the feature with autosave parameter only recently. Do the drivers for other languages like Python, Go, JavaScript have the feature? We should reduce the burdon on the driver developers. Fourth, the runtime performance. In a performance benchmark of one of our customers, where a batch application ran 1.5 or 5 million small SELECTs with primary key access, the execution time of the whole batch became shorter by more than 30% (IIRC) when the local connection was used instead of the remote TCP/IP one. The communication overhead is not small. Also, in the PostgreSQL documentation, the communication overhead is treated seriously as follows: https://www.postgresql.org/docs/devel/static/plpgsql-overview.html#plpgsql-advantages [Excerpt] -- That means that your client application must send each query to the database server, wait for it to be processed, receive and process the results, do some computation, then send further queries to the server. All this incurs interprocess communication and will also incur network overhead if your client is on a different machine than the database server. With PL/pgSQL you can group a block of computation and a series of queries inside the database server, thus having the power of a procedural language and the ease of use of SQL, but with considerable savings of client/server communication overhead. •Extra round trips between client and server are eliminated •Intermediate results that the client does not need do not have to be marshaled or transferred between server and client •Multiple rounds of query parsing can be avoided This can result in a considerable performance increase as compared to an application that does not use stored functions. -- Craig reports the big communication overhead: PATCH: Batch/pipelining support for libpq https://www.postgresql.org/message-id/CAMsr+YFUjJytRyV4J-16bEoiZyH=4nj+sQ7JP9ajwz=b4dm...@mail.gmail.com#CAMsr+YFUjJytRyV4J-16bEoiZyH=4nj+sQ7JP9ajwz=b4dm...@mail.gmail.com Re: foreign table batch insert https://www.postgresql.org/message-id/CAMsr+YFgDUiJ37DEfPRk8WDBuZ58psdAYJd8iNFSaGxtw=w...@mail.gmail.com [Excerpt] -- The time difference for 10k inserts on the local host over a unix socket shows a solid improvement: batch insert elapsed: 0.244293s sequential insert elapsed: 0.375402s ... but over, say, a connection to a random
Re: [HACKERS] [bug fix] postgres.exe crashes with access violation on Windows while starting up
From: Michael Paquier [mailto:michael.paqu...@gmail.com] > On Tue, Oct 31, 2017 at 6:59 AM, Tsunakawa, Takayuki > <tsunakawa.ta...@jp.fujitsu.com> wrote: > > When CurrentMemoryContext is NULL, the message is logged with > ReportEventA(). This is similar to write_console(). > > My point is that as Postgres is running as a service, isn't it wrong to > write a message to stderr as a fallback if the memory context is not set? > You would lose a message. It seems to me that for an operation that can > happen at a low-level like the postmaster startup, we should really use > a low-level operation as well. I'm sorry I may not have been clear. With this patch, write_eventlog() outputs the message to event log with ReportEventA() when CurrentMemoryContext is NULL. It doesn't write to stderr. So the message won't be lost. Regards Takayuki Tsunakawa -- 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] [bug fix] postgres.exe crashes with access violation on Windows while starting up
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Paquier > So you are basically ready to lose any message that could be pushed > here if there is no memory context? That does not sound like a good > trade-off to me. A static buffer does not look like the best idea > either to not truncate message, so couldn't we envisage to just use > malloc? pgwin32_message_to_UTF16() is called in two places in elog.c, > and there is a full control on the error code paths. Thank you for reviewing a rare bug fix on Windows that most people wouldn't be interested in. When CurrentMemoryContext is NULL, the message is logged with ReportEventA(). This is similar to write_console(). Regards Takayuki Tsunakawa -- 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] proposal: schema variables
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Pavel Stehule > I propose a new database object - a variable. The variable is persistent > object, that holds unshared session based not transactional in memory value > of any type. Like variables in any other languages. The persistence is > required for possibility to do static checks, but can be limited to session > - the variables can be temporal. > > > My proposal is related to session variables from Sybase, MSSQL or MySQL > (based on prefix usage @ or @@), or package variables from Oracle (access > is controlled by scope), or schema variables from DB2. Any design is coming > from different sources, traditions and has some advantages or disadvantages. > The base of my proposal is usage schema variables as session variables for > stored procedures. It should to help to people who try to port complex > projects to PostgreSQL from other databases. Very interesting. I hope I could join the review and testing. How do you think this would contribute to easing the port of Oracle PL/SQL procedures? Would the combination of orafce and this feature promote auto-translation of PL/SQL procedures? I'm curious what will be the major road blocks after adding the schema variable. Regards Takayuki Tsunakawa -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [bug fix] postgres.exe crashes with access violation on Windows while starting up
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tsunakawa, > Takayuki > The reason is for not outputing the crash dump is a) the crash occurred > before installing the Windows exception handler > (pgwin32_install_crashdump_handler() call) and b) the effect of the > following call in postmaster is inherited in the child process. > > /* In case of general protection fault, don't show GUI popup > box */ > SetErrorMode(SEM_FAILCRITICALERRORS | > SEM_NOGPFAULTERRORBOX); > > But I'm not sure in what order we should do > pgwin32_install_crashdump_handler(), startup_hacks() and steps therein, > MemoryContextInit(). I think that's another patch. Just installing the handler at the beginning of main() seems fine. Patch attached. Regards Takayuki Tsunakawa crash_dump_before_installing_handler.patch Description: crash_dump_before_installing_handler.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [bug fix] postgres.exe crashes with access violation on Windows while starting up
Hello, We encountered a rare and hard-to-investigate problem on Windows, which one of our customers reported. Please find the attached patch to fix that. I'll add this to the next CF. PROBLEM == PostgreSQL sometimes crashes with the following messages. This is infrequent (but frequent for the customer); it occurred about 10 times in the past 5 months. LOG: server process (PID 2712) was terminated by exception 0xC005 HINT: See C include file "ntstatus.h" for a description of the hexadecimal value. LOG: terminating any other active server processes WARNING: terminating connection because of crash of another server process DETAIL: The postmaster has commanded this server process to roll back the current transaction and exit, because another server process exited abnormally and possibly corrupted shared memory. HINT: In a moment you should be able to reconnect to the database and repeat your command. LOG: all server processes terminated; reinitializing "server process" shows that an client backend crashed. The above messages indicate that the process was not running an SQL command. PostgreSQL runs as a Windows service. No crash dump was produced anywhere, despite the facts: - /crashdumps folder exists and is writable by the PostgreSQL user account (which is the user postgres.exe runs as) - The Windows registry configuration allows dumping the crash dump CAUSE == We believe WSAStartup() in main.c failed. The only conceivable error is: WSAEPROCLIM 10067 Too many processes. A Windows Sockets implementation may have a limit on the number of applications that can use it simultaneously. WSAStartup may fail with this error if the limit has been reached. But I couldn't find what the limit is and whether we can tune it. We couldn't reproduce the problem. When I pretend that WSAStartup() failed while a client backend is starting up, I could see the same phenomenon as the customer. This problem only occurs when PostgreSQL runs as a Windows service. The bug is in write_eventlog(). It calls pgwin32_message_to_utf16() which in turn calls palloc(), which requires the memory management system to be set up (CurrentMemoryContext != NULL). FIX == Add the check "CurrentMemoryContext != NULL" in write_eventlog() as in write_console(). NOTE == The reason is for not outputing the crash dump is a) the crash occurred before installing the Windows exception handler (pgwin32_install_crashdump_handler() call) and b) the effect of the following call in postmaster is inherited in the child process. /* In case of general protection fault, don't show GUI popup box */ SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX); But I'm not sure in what order we should do pgwin32_install_crashdump_handler(), startup_hacks() and steps therein, MemoryContextInit(). I think that's another patch. Regards Takayuki Tsunakawa write_eventlog_crash.patch Description: write_eventlog_crash.patch -- 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] Remove secondary checkpoint
From: Alvaro Herrera [mailto:alvhe...@alvh.no-ip.org] > Tsunakawa, Takayuki wrote: > > > (Although unrelated to this, I've also been wondering why PostgreSQL > > flushes WAL to disk when writing a page in the shared buffer, because > > PostgreSQL doesn't use WAL for undo.) > > The reason is that if the system crashes after writing the data page to > disk, but before writing the WAL, the data page would be inconsistent with > data in pages that weren't flushed, since there is no WAL to update those > other pages. Also, if the system crashes after partially writing the page > (say it writes the first 4kB) then the page is downright corrupted with > no way to fix it. > > So there has to be a barrier that ensures that the WAL is flushed up to > the last position that modified a page (i.e. that page's LSN) before actually > writing that page to disk. And this is why we can't use mmap() for shared > buffers -- there is no mechanism to force the WAL down if the operation > system has the liberty to flush pages whenever it likes. I see. The latter is a torn page problem, which is solved by a full page image WAL record. I understood that an example of the former problem is the inconsistency between a table page and an index page -- if an index page is flushed to disk without slushing the WAL and the corresponding table page, an index entry would point to a wroing table record after recovery. Thanks, my long-standing question has beenn solved. Regards Takayuki Tsunakawa -- 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] [bug fix] ECPG: fails to recognize embedded parameters
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Meskes > Thanks for spotting and fixing. I just committed your patch to master and > backported to 9.4, 9.5, 9.6 and 10. It doesn't apply cleanly to 9.3. But > then it might not be important enough to investigate and backported to this > old a version. Thanks. I'm OK with 9.3. Regards Takayuki Tsunakawa -- 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] Remove secondary checkpoint
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Simon Riggs > This > * reduces disk space requirements on master > * removes a minor bug in fast failover > * simplifies code I welcome this patch. I was wondering why PostgreSQL retains the previous checkpoint. (Although unrelated to this, I've also been wondering why PostgreSQL flushes WAL to disk when writing a page in the shared buffer, because PostgreSQL doesn't use WAL for undo.) Here are my review comments. (1) -* a) we keep WAL for two checkpoint cycles, back to the "prev" checkpoint. +* a) we keep WAL for only one checkpoint cycle (prior to PG11 we kept +*WAL for two checkpoint cycles to allow us to recover from the +*secondary checkpoint if the first checkpoint failed, though we +*only did this on the master anyway, not on standby. Keeping just +*one checkpoint simplifies processing and reduces disk space in +*many smaller databases.) /* -* The last valid checkpoint record required for a streaming -* recovery exists in neither standby nor the primary. +* We used to attempt to go back to a secondary checkpoint +* record here, but only when not in standby_mode. We now +* just fail if we can't read the last checkpoint because +* this allows us to simplify processing around checkpoints. */ if (fast_promote) { - checkPointLoc = ControlFile->prevCheckPoint; + /* +* It appears to be a bug that we used to use prevCheckpoint here +*/ + checkPointLoc = ControlFile->checkPoint; - XLByteToSeg(PriorRedoPtr, _logSegNo, wal_segment_size); + /* Trim from the last checkpoint, not the last - 1 */ + XLByteToSeg(RedoRecPtr, _logSegNo, wal_segment_size); I suggest to remove references to the secondary checkpoint (the old behavior) from the comments. I'm afraid those could confuse new developers. (2) @@ -8110,10 +8100,6 @@ ReadCheckpointRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, ereport(LOG, (errmsg("invalid primary checkpoint link in control file"))); break; @@ -8135,10 +8121,6 @@ ReadCheckpointRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, ereport(LOG, (errmsg("invalid primary checkpoint record"))); break; @@ -8154,10 +8136,6 @@ ReadCheckpointRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, ereport(LOG, (errmsg("invalid resource manager ID in primary checkpoint record"))); break; etc, etc. "primary checkpoint" should be just "checkpoint", now that the primary/secondary concept will disappear. (3) Should we change the default value of max_wal_size from 1 GB to a smaller size? I vote for "no" for performance. Regards Takayuki Tsunakawa -- 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] Remove secondary checkpoint
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Paquier > On Tue, Oct 24, 2017 at 5:58 PM, Tsunakawa, Takayuki > <tsunakawa.ta...@jp.fujitsu.com> wrote: > > If the latest checkpoint record is unreadable (the WAL > segment/block/record is corrupt?), recovery from the previous checkpoint > would also stop at the latest checkpoint. And we don't need to replay the > WAL records between the previous checkpoint and the latest one, because > their changes are already persisted when the latest checkpoint was taken. > So, the user should just do pg_resetxlog and start the database server when > the recovery cannot find the latest checkpoint record and PANICs? > > Not necessarily. If a failure is detected when reading the last checkpoint, > as you say recovery would begin at the checkpoint prior that and would stop > when reading the record of last checkpoint, still one could use a > recovery.conf with restore_command to fetch segments from a different > source, like a static archive, as only the local segment may be corrupted. Oh, you are right. "If the crash recovery fails, perform recovery from backup." Anyway, I agree that the secondary checkpoint isn't necessary. Regards Takayuki Tsunakawa -- 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] Remove secondary checkpoint
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tom Lane > Doesn't it also make crash recovery less robust? The whole point > of that mechanism is to be able to cope if the latest checkpoint > record is unreadable. If the latest checkpoint record is unreadable (the WAL segment/block/record is corrupt?), recovery from the previous checkpoint would also stop at the latest checkpoint. And we don't need to replay the WAL records between the previous checkpoint and the latest one, because their changes are already persisted when the latest checkpoint was taken. So, the user should just do pg_resetxlog and start the database server when the recovery cannot find the latest checkpoint record and PANICs? Regards Takayuki Tsunakawa -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [bug fix] ECPG: fails to recognize embedded parameters
Hello, This is an actual problem that our customer hit. In ECPG, opening a cursor fails which is declared as follows: EXEC SQL DECLARE cur CURSOR FOR SELECT oid, datname FROM pg_database WHERE datname LIKE 'post%' ESCAPE '\' AND datconnlimit = :connlimit; sqlstate: 07001 sqlerrm.sqlerrmc: too many arguments on line 30 The cause is that next_insert() in ecpglib unconditionally skips the next character after the backslash. Could you review and commit the attached patch? I also attached the test program for convenience. Regards Takayuki Tsunakawa ecpg_escape_string.patch Description: ecpg_escape_string.patch escape.ec Description: escape.ec -- 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] list of credits for release notes
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Peter Eisentraut > At the PGCon Developer Meeting it was agreed[0] to add a list of credits > to the release notes, including everyone who was mentioned in a commit > message. I have now completed that list. Thank you for your hard work. "Daisuke Higuchi" and "Higuchi Daisuke" are the same person (my colleague). Please use the former. Regards Takayuki Tsunakawa -- 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] PG 10 release notes
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tom Lane > TBH, I think that's another reason for not release-noting it. There's no > concrete change to point to, and so it's hard to figure out what to say. > I'm not even very sure that we should be encouraging people to change > existing shared_buffers settings; the experiments that Takayuki-san did > were only on Windows 10, so we don't really know that changing that would > be a good idea on older Windows versions. In fact, when I ran pgbench on Windows 2008 for some other unrelated reason, I found larger shared buffers (4GB, 8GB) was effective, too. I didn't know pure documentation changes are not listed on the release notes. But I suggest listing them (of course, excluding mere typos), because the documentation is also important for users as well as programs. The documentation is also part of software product. If the documented behavior and the actual one differs and the user is wondering which is correct, he can know the answer only from the release note when the mismatch is fixed. I think the documentation changes are more useful for users than, for example, the following items: E.1.3.11. Source Code Improve behavior of pgindent (Piotr Stefaniak, Tom Lane) Allow WaitLatchOrSocket() to wait for socket connection on Windows (Andres Freund) Overhaul documentation build process (Alexander Lakhin) Use XSLT to build the PostgreSQL documentation (Peter Eisentraut) Regards Takayuki Tsunakawa -- 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] sync process names between ps and pg_stat_activity
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Peter Eisentraut > > Personally, I prefer "wal writer", "wal sender" and "wal receiver" > > that separate words as other process names. But I don't mind leaving > > them as they are now. > > If we were to change those, that would break existing queries for > pg_stat_activity. That's new in PG10, so we could change it if we were > really eager. But it's probably not worth bothering. Then again, there > is pg_stat_wal_receiver. So it's all totally inconsistent. Not sure > where to go. OK, I'm comfortable with as it is now. I made this ready for committer. You can fix the following and commit the patch. Thank you. > > * To achieve that, we pass "wal sender process" as username and > > username > > good catch Regards Takayuki Tsunakawa -- 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] Re: [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur
Hello Tom, Michael, Robert, Noah From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas > On Thu, Sep 14, 2017 at 3:23 AM, Tsunakawa, Takayuki > <tsunakawa.ta...@jp.fujitsu.com> wrote: > > Sorry again, but how can we handle this? A non-PG-developer, Tels (and > possibly someone else, IIRC) claimed that the behavior be changed during > the beta period. Why should we do nothing? > > Because we do not have consensus on changing it. I've decided that you're > right, but several other people are saying "no". I can't just go change > it in the face of objections. How are things decided in cases like this? Does the RMT team usually do some kind of poll? So far, there are four proponents (Tels (non-PG-developer), David J., Robert and me), and two opponents (Tom and Michael). Regards Takayuki Tsunakawa -- 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] Process startup infrastructure is a mess
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Andres Freund > I think we should seriously consider doing a larger refactoring of this > soon. I've some ideas about what to do, but I'd welcome some thoughts on > whether others consider this a serious problem or not, and what they think > we should do about this, first. Thank you for raising this. I think so, too. Some other things that quickly come to mind are: * The signal handlers are similar in many processes and redundant. It's not easy to see how the processes respond to a particular signal and what signal can be used for new things such as dumping the stack trace in the server log. Maybe we can have an array of process attributes that specify the signal handlers, startup/shutdown order, etc. Then the common process management code handles processes based on the information in the array. * postmaster.c is very large (more than 6,000 lines) and hard to read. * It may be easy to forget adding initialization code in the Windows-specific path (SubPostmasterMaain). * It's confusing to find out which process counts toward max_connections, MaxBackends, the count of auxiliary processes. As a user, I'm sometimes confused about the relationship between max_connections, autovacuum_max_workers, max_wal_senders, max_worker_processes. Regards Takayuki Tsunakawa -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur
Hello, Robert, Noah From: Robert Haas [mailto:robertmh...@gmail.com] > On Fri, Jul 28, 2017 at 1:30 AM, Noah Mischwrote: > > We've reached that period. If anyone is going to push for a change > > here, now is the time. Absent such arguments, the behavior won't change. > > Well, I started out believing that the current behavior was for the best, > and then completely reversed my position and favored the OP's proposal. > Nothing has really happened since then to change my mind, so I guess I'm > still in that camp. But do we have any new data points? Have any > beta-testers tested this and what do they think? > The only non-developer (i.e. person not living in an ivory tower) who has > weighed in here is Tels, who favored reversing the original decision and > adopting Tsunakawa-san's position, and that was 2 months ago. > > I am pretty reluctant to tinker with this at this late date and in the face > of several opposing votes, but I do think that we bet on the wrong horse. Sorry again, but how can we handle this? A non-PG-developer, Tels (and possibly someone else, IIRC) claimed that the behavior be changed during the beta period. Why should we do nothing? Regards Takayuki Tsunakawa -- 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] PG 10 release notes
It's embarrassing to ask about such a trivial thing, but I noticed the following line was missing in the latest release note, which was originally in Bruce's website: Remove documented restriction about using large shared buffers on Windows (Takayuki Tsunakawa) Is this intended? Regards Takayuki Tsunakawa -- 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] Supporting huge pages on Windows
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Ashutosh Sharma > I have once again tested the latest patch (v14 patch) on Windows and the > results looked fine to me. Basically I have repeated the test cases which > I had done earlier on v8 patch. For more details, on the tests that i have > re-executed, please refer to - [1]. Thanks. Thanks so much. I'm relieved to know that the patch still works. Regards Takayuki Tsunakawa -- 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] [bug fix] Savepoint-related statements terminates connection
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tom Lane > The originally reported bug is fixed. Not making any claims about other > bugs ... I'm sorry I couldn't reply to you. I've recently been in a situation where I can't use my time for development. I think I'll be able to rejoin the community activity soon. I confirmed your patch fixed the problem. And the code looks perfect. Thank you very much. Regards Takayuki Tsunakawa -- 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] Supporting huge pages on Windows
Hi Thomas, Magnus From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Thomas Munro > Since it only conflicts with c7b8998e because of pgindent whitespace > movement, I applied it with "patch -p1 --ignore-whitespace" and created > a new patch. See attached. Thanks, Thomas. I've added your name in the CF entry so that your name will also be listed on the release note, because my patch is originally based on your initial try. Please remove your name just in case you mind it. BTW, your auto-reviewer looks very convenient. Thank you again for your great work. Magnus, it would be grateful if you could review and commit the patch while your memory is relatively fresh. I've been in a situation which keeps me from doing development recently, but I think I can gradually rejoin the community activity soon. Regards Takayuki Tsunakawa -- 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] How can I find a specific collation in pg_collation when using ICU?
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Peter Eisentraut > There are no case-insensitive collations in PostgreSQL (yet). That's sad news, as I expected ICU to bring its various collation features to PostgreSQL. I hope it will be easy to add them. From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Peter Geoghegan > This is not an answer to the question you asked, but it may interest you > to know that ICU uses JIS X 4061 for Japanese, unlike Glibc. This will give > more useful results when sorting Japanese. > > The best explanation of the difference that I can understand is here, under > "Why do CJK strings sort incorrectly in Unicode?": > > https://dev.mysql.com/doc/refman/5.5/en/faqs-cjk.html Thanks a lot. MysQL seems to have many collations, doesn't it? Regards Takayuki Tsunakawa -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur
From: Robert Haas [mailto:robertmh...@gmail.com] > Well, I started out believing that the current behavior was for the best, > and then completely reversed my position and favored the OP's proposal. > Nothing has really happened since then to change my mind, so I guess I'm > still in that camp. But do we have any new data points? Have any > beta-testers tested this and what do they think? > The only non-developer (i.e. person not living in an ivory tower) who has > weighed in here is Tels, who favored reversing the original decision and > adopting Tsunakawa-san's position, and that was 2 months ago. > > I am pretty reluctant to tinker with this at this late date and in the face > of several opposing votes, but I do think that we bet on the wrong horse. Thank you Robert and Tels. Yes, Tels's comment sounds plausible as a representative real user who expects high availability. I'm sorry to repeat myself, but this feature is for HA, so libpq should attempt to connect to the next host when it fails to establish a connection. Who can conclude this? I don't think that no feedback from beta users means satisfaction with the current behavior. Regards Takayuki Tsunakawa -- 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] Huge pages support on Windows
Hello, Andrea From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Andrea caldarone > Anyone can point me in the right direction? Thank you for your interest. 1. Download the PostgreSQL 10 source code (which is still in development), which is the file named postgresql-snapshot.tar.gz from here: https://ftp.postgresql.org/pub/snapshot/dev/ 2. Get the latest patch for huge page support for Windows in the mail thread you saw. The file name is win_large_pages_v13.patch. 3. Decompress the source code and apply the patch. $ tar zxf postgresql-snapshot.tar.gz $ cd postgresql* $ patch -p1 < win_large_pages_v13.patch Now you can build and install the program normally. Regards Takayuki Tsunakawa -- 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] Is ECPG's SET CONNECTION really not thread-aware?
Dear Meskes, From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Meskes > Done. Thanks, I confirmed the commit messages. > My standard workflow is to wait a couple days to see if everything works > nicely before backporting. Obviously this doesn't make any sense for a doc > patch, sigh. I see. That's a good idea. Regards Takayuki Tsunakawa -- 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] Is ECPG's SET CONNECTION really not thread-aware?
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Meskes > I'm pretty sure it is indeed thread-aware, although I didn't provide the > code for this feature myself. > > > So the doc seems to need fix. The patch is attached. > > Thanks, applied. Thank you, I'm relieved. Could you also apply it to past versions if you don't mind? The oldest supported version 9.2 is already thread-aware. Regards Takayuki Tsunakawa -- 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] Is ECPG's SET CONNECTION really not thread-aware?
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tsunakawa, > Takayuki > The following page says: > > https://www.postgresql.org/docs/devel/static/ecpg-connect.html#ecpg-se > t-connection > > -- > EXEC SQL AT connection-name SELECT ...; > > If your application uses multiple threads of execution, they cannot share > a connection concurrently. You must either explicitly control access to > the connection (using mutexes) or use a connection for each thread. If each > thread uses its own connection, you will need to use the AT clause to specify > which connection the thread will use. > > EXEC SQL SET CONNECTION connection-name; > > ...It is not thread-aware. > -- > > > What does this mean by "not thread-aware?" Does SET CONNECTION in one > thread change the current connection in another thread? > It doesn't look so, because the connection management and SQL execution > in ECPG uses thread-specific data (TSD) to get and set the current connection, > like this: The ECPG code sets and gets the current connection to/from the TSD, so SET CONNECTION is thread-aware. I could confirm that like this: threadA: CONNECT AS conA threadB: CONNECT AS conB threadA: CONNECT AS conC threadA & threadB: SELECT pg_backend_pid() ... each thread displays different pids threadA: SET CONNECTION conA threadA & threadB: SELECT pg_backend_pid() ... each thread displays different pids, with thread outputting the same pid as before So the doc seems to need fix. The patch is attached. Regards Takayuki Tsunakawa ecpg_setconn.patch Description: ecpg_setconn.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Is ECPG's SET CONNECTION really not thread-aware?
Hello, The following page says: https://www.postgresql.org/docs/devel/static/ecpg-connect.html#ecpg-set-connection -- EXEC SQL AT connection-name SELECT ...; If your application uses multiple threads of execution, they cannot share a connection concurrently. You must either explicitly control access to the connection (using mutexes) or use a connection for each thread. If each thread uses its own connection, you will need to use the AT clause to specify which connection the thread will use. EXEC SQL SET CONNECTION connection-name; ...It is not thread-aware. -- What does this mean by "not thread-aware?" Does SET CONNECTION in one thread change the current connection in another thread? It doesn't look so, because the connection management and SQL execution in ECPG uses thread-specific data (TSD) to get and set the current connection, like this: bool ECPGsetconn(int lineno, const char *connection_name) { struct connection *con = ecpg_get_connection(connection_name); if (!ecpg_init(con, connection_name, lineno)) return (false); #ifdef ENABLE_THREAD_SAFETY pthread_setspecific(actual_connection_key, con); #else actual_connection = con; #endif return true; } Regards Takayuki Tsunakawa -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: retry shm attach for windows (WAS: Re: [HACKERS] OK, so culicidae is *still* broken)
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Amit Kapila > Yes, I also share this opinion, the shm attach failures are due to > randomization behavior, so sleep won't help much. So, I will change the > patch to use 100 retries unless people have other opinions. Perhaps I'm misunderstanding, but I thought it is not known yet whether the cause of the original problem is ASLR. I remember someone referred to anti-virus software and something else. I guessed that the reason Noah suggested 1 - 5 seconds of retry is based on the expectation that the address space might be freed by the anti-virus software. Regards Takayuki Tsunakawa -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: retry shm attach for windows (WAS: Re: [HACKERS] OK, so culicidae is *still* broken)
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Noah Misch > Ten feels low to me. The value should be be low enough so users don't give > up and assume a permanent hang, but there's little advantage to making it > lower. > I'd set it such that we give up in 1-5s on a modern Windows machine, which > I expect implies a retry count of one hundred or more. Then, maybe we can measure the time in each iteration and give up after a particular seconds. Regards Takayuki Tsunakawa -- 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 v1] Add and report the new "in_hot_standby" GUC pseudo-variable.
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tom Lane > I didn't look at exactly how you tried to do that, but GUCs whose values > depend on other GUCs generally don't work well at all. transaction_read_only and transaction_isolation depends on default_transaction_read_only and default_transaction_isolation respectively. But I feel you are concerned about something I'm not aware of. Could you share your worries? I haven't found a problem so far. > >> * Its value is false during recovery. > > [ scratches head... ] Surely this should read as "true" during recovery? Ouch, you are right. > Also, what happens if the standby server goes live mid-session? The clients will know the change of session_read_only when they do something that calls RecoveryInProgress(). Currently, RecoveryInProgress() seems to be the only place where the sessions notice the promotion, so I set session_read_only to the value of default_transaction_read_only there. I think that there is room for discussion here. It would be ideal for the sessions to notice the server promotion promptly and notify the clients of the change. I have no idea to do that well. Regards Takayuki Tsunakawa -- 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 v1] Add and report the new "in_hot_standby" GUC pseudo-variable.
From: Robert Haas [mailto:robertmh...@gmail.com] > I've already stated my position on this, which is that: > > * target_session_attrs=read-only is a perfectly good new feature, but we're > past feature freeze, so it's material for v11. > > * I'm not opposed to adding a GUC_REPORT GUC of some kind, but I see no > urgency about that either. The feature works fine as it is. The fact that > it could possibly be made to work more efficiently is not a critical bug. I see. I'll add this in the next CF for PG 11. I'd be glad if you could review the patch in PG 11 development. Also, I'll update the PostgreSQL 10 Open Items page that way. Regards Takayuki Tsunakawa -- 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 v1] Add and report the new "in_hot_standby" GUC pseudo-variable.
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Craig Ringer > On 13 April 2017 at 14:59, Tsunakawa, Takayuki > <tsunakawa.ta...@jp.fujitsu.com> wrote: > > > 2. Make transaction_read_only GUC_REPORT This is to avoid the added > > round-trip by SHOW command. It also benefits client apps that want to > know when the server gets promoted? And this may simplify the libpq code. > > I don't understand yet why we need to provide this feature for older servers > by using SHOW. Those who are already using <= 9.6 in production completed > the system or application, and their business is running. Why would they > want to just replace libpq and use this feature? > > I think "transaction_read_only" is a bit confusing for something we're > expecting to change under us. > > To me, a "read only" xact is one created with > > BEGIN READ ONLY TRANSACTION; > > which I would not expect to become read/write under me, since I > explicitly asked for read-only. > > It's more like "session read only" that we're interested in IMO. I confirmed that the attached patch successfully provides: * target_session_attrs=read-only * If the server is >= 10, avoid the round-trip for SHOW transaction_read_only. For this, I added a GUC_REPORT variable session_read_only which indicates the session's default read-only status. The characteristics are: * It cannot be changed directly by the user (postgresql.conf, SET, etc.) * Its value is the same as default_transaction_read_only when not in recovery. * Its value is false during recovery. Could you include this in PG 10? I think these are necessary as the bottom line to meet the average expectation of users (please don't ask me what's the average; the main reasons are that PostgreSQL provides hot standby, PgJDBC enables connection to the standby (targetServerType=slave), and PostgreSQL emphasizes performance.) Ideally, I wanted to add other features of PgJDBC (e.g. targetServerType=preferSlave), but I thought this is the limit not to endanger the quality of the final release. Regards Takayuki Tsunakawa libpq-fast-connect-read-only.patch Description: libpq-fast-connect-read-only.patch -- 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] Re: [doc fix] PG10: wroing description on connect_timeout when multiple hosts are specified
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas > Committed. I also added a slight tweak to the wording of the documentation. Thank you, the doc looks clearer. Regards Takayuki Tsunakawa -- 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] [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Paquier > > One thing I'm worried is that people here might become more conservative > against change once the final version is released. > > Any redesign after release would finish by being a new feature, which would > be in this case a new connection parameter or an extra option that works > with the current parameter, say something to allow soft or hard failures > when multiple hosts are defined. Hmm... but I can't imagine the parameter would be very meaningful for users. Regards Takayuki Tsunakawa -- 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] [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Paquier > On Thu, May 18, 2017 at 11:30 PM, Robert Haaswrote: > > Because why? > > Because it is critical to let the user know as well *why* an error happened. > Imagine that this feature is used with multiple nodes, all primaries. If > a DB admin busted the credentials in one of them then all the load would > be redirected on the other nodes, without knowing what is actually causing > the error. Then the node where the credentials have been changed would just > run idle, and the application would be unaware of that. In that case, the DBA can know the authentication errors in the server log of the idle instance. I'm sorry to repeat myself, but libpq connection failover is the feature for HA. So I believe what to prioritize is HA. And the documentation looks somewhat misleading. I get the impression that libpq tries hosts until success regardless of failure reason: it aims for successful connection, not giving up early. Who can read this as "libpq gives up connection under some circumstances?" https://www.postgresql.org/docs/devel/static/libpq-connect.html -- It is possible to specify multiple host components, each with an optional port component, in a single URI. A URI of the form postgresql://host1:port1,host2:port2,host3:port3/ is equivalent to a connection string of the form host=host1,host2,host3 port=port1,port2,port3. Each host will be tried in turn until a connection is successfully established. ... If multiple host names are specified, each will be tried in turn in the order given. -- Regards Takayuki Tsunakawa -- 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] [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Peter Eisentraut > The problem is that if we decide to change the behavior mid-beta, then we'll > only have the rest of beta to find out whether people will like the other > behavior. > > I would aim for the behavior that is most suitable for refinement in the > future. The current behavior seems to match that. I think the pre-final release period is the very timing for refinement, in the perspective of users and PG developers as users. One thing I'm worried is that people here might become more conservative against change once the final version is released. Regards Takayuki Tsunakawa -- 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] [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tsunakawa, > Takayuki > Done. I'll examine whether we can use SQLSTATE. I tried conceivable errors during connection. Those SQLSTATEs are as follows: [transient error (after which you may want to try next host)] 53300 FATAL: too many connections for role "tuna" 57P03 FATAL: the database system is starting up [configuration error (after which you may give up connection without other hosts. Really?)] 55000 FATAL: database "template0" is not currently accepting connections 3D000 FATAL: database "aaa" does not exist 28000 FATAL: no pg_hba.conf entry for host "::1", user "tunakawa", database "postgres", SSL off 28000 FATAL: role "nouser" does not exist 28P01 FATAL: password authentication failed for user "tuna" 28P01 DETAIL: Password does not match for user "tuna". I looked through the SQLSTATEs, and thought the below ones could possibly be returned during connection: https://www.postgresql.org/docs/devel/static/errcodes-appendix.html [transient error] Class 08 - Connection Exception Class 40 - Transaction Rollback Class 53 - Insufficient Resources Class 54 - Program Limit Exceeded Class 55 - Object Not In Prerequisite State Class 57 - Operator Intervention Class 58 - System Error (errors external to PostgreSQL itself) Class XX - Internal Error [configuration error] Class 28 - Invalid Authorization Specification Class 3D - Invalid Catalog Name Class 42 - Syntax Error or Access Rule Violation So, how about trying connection to the next host when the class code is neither 28, 3D, nor 42? Honestly, I'm not happy with this approach, for a maintenance reason that others are worried about. Besides, when the connection target is not postgres and returns invalid data, no SQLSTATE is available. I'm sorry to repeat myself, but I believe PgJDBC's approach is practically good. If you think the SQLSTATE is the only way to go, I will put up with it. It would be disappointing if nothing is done. Regards Takayuki Tsunakawa -- 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] [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur
From: Michael Paquier [mailto:michael.paqu...@gmail.com] > Does JDBC use something like that to make a difference between a failure > and a move-on-to-next-one? No, it just tries the next host. See the first while loop in org/postgresql/jdbc/core/v3/ConnectionFactoryImpl.java. > From maintenance point of view, this would > require lookups each time a new SQLSTATE is added. Not sure that people > would remember that. Yes, I have the same concern, but I'll see if there's a good way anyway (e.g. whether we can simply use the class code of the SQLSTATE, which seems hopeless.) I guess PgJDBC's way is practical and sensible in the end. Regards Takayuki Tsunakawa -- 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] [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur
From: Tom Lane [mailto:t...@sss.pgh.pa.us] > Sure, but part of the point of beta testing is to get user feedback. Yes, and I'm also proposing this in the user's point of view, which I believe holds true for people here. I'm worried from my support experience that strict customers would complain and question the HA. > I agree with Robert's point that major redesign of the feature on the basis > of one complaint isn't necessarily the way to go. Since the existing > behavior is already out in beta1, let's wait and see if anyone else complains. > We don't need to fix it Right This Instant. I'm OK with considering this during beta testing. But do you think there will be enough beta testers and some of them will find this kind of subtle problem? I'm afraid this type of problem will be detected and complained after some time in production... So, I think we should address this proactively on the basis of good sense. > Maybe add this to the list of open issues to reconsider mid-beta? Done. I'll examine whether we can use SQLSTATE. Regards Takayuki Tsunakawa -- 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] [bug fix] Savepoint-related statements terminates connection
From: Michael Paquier [mailto:michael.paqu...@gmail.com] > On Fri, Mar 31, 2017 at 9:58 PM, Ashutosh Bapat >wrote: > > Then the question is why not to allow savepoints as well? For that we > > have to fix transaction block state machine. > > I agree with this argument. I have been looking at the patch, and what it > does is definitely incorrect. Any query string including multiple queries > sent to the server is executed as a single transaction. So, while the current > behavior of the server is definitely incorrect for savepoints in this case, > the proposed patch does not fix anything but actually makes things worse. > I think that instead of failing, savepoints should be able to work properly. > As you say cursors are handled correctly, savepoints should fall under the > same rules. Yes, I'm in favor of your opinion. I'll put more thought into whether it's feasible with invasive code. Regards Takayuki Tsunakawa -- 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] [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur
Hello Robert, Tom, Thank you for being kind enough to explain. I think I could understand your concern. From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas > Who is right is a judgement call, but I don't think it's self-evident that > users want to ignore anything and everything that might have gone wrong > with the connection to the first server, rather than only those things which > resemble a down server. It seems quite possible to me that if we had defined > it as you are proposing, somebody would now be arguing for a behavior change > in the other direction. Judgment call... so, I understood that it's a matter of choosing between helping to detect configuration errors early or service continuity. Hmm, I'd like to know how other databases treat this, but I couldn't find useful information after some Google search. I wonder whether I sould ask PgJDBC people if they know something, because they chose service continuity. From: Tom Lane [mailto:t...@sss.pgh.pa.us] > The bigger picture here is that we only want to fail past transient errors, > not configuration errors. I'm willing to err in favor of regarding doubtful > cases as transient, but most server login rejections aren't for transient > causes. I got "doubtful cases" as ones such as specifying non-existent host or an unused port number. In that case, the configuration error can't be distinguished from the server failure. What do you think of the following cases? Don't you want to connect to other servers? * The DBA shuts down the database. The server takes a long time to do checkpointing. During the shutdown checkpoint, libpq tries to connect to the server and receive an error "the database system is shutting down." * The former primary failed and now is trying to start as a standby, catching up by applying WAL. During the recovery, libpq tries to connect to the server and receive an error "the database system is performing recovery." * The database server crashed due to a bug. Unfortunately, the server takes unexpectedly long time to shut down because it takes many seconds to write the stats file (as you remember, Tom-san experienced 57 seconds to write the stats file during regression tests.) During the stats file write, libpq tries to connect to the server and receive an error "the database system is shutting down." These are equivalent to server failure. I believe we should prioritize rescuing errors during operation over detecting configuration errors. > Of course, the user would have to try connections to both foo and bar to > be sure that they're both configured correctly. But he might try > "host=foo,bar" and "host=bar,foo" and figure he was OK, not noticing that > both connections had silently been made to bar. In that case, I think he would specify "host=foo" and "host=bar" in turn, because he would be worried about where he's connected if he specified multiple hosts. Regards Takayuki Tsunakawa -- 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] Re: [doc fix] PG10: wroing description on connect_timeout when multiple hosts are specified
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Paquier > pqWait is internal to libpq, so we are free to set up what we want here. > Still I think that we should be consistent with what pqSocketCheck returns: Please let this what it is now for the same reason Robert mentioned. > +intret = 0; > +inttimeout = 0; > The declaration of "ret" should be internal in the for(;;) loop. Done. > + /* Attempt connection to the next host, starting the > connect_timeout timer */ > + pqDropConnection(conn, true); > + conn->addr_cur = conn->connhost[conn->whichhost].addrlist; > + conn->status = CONNECTION_NEEDED; > + finish_time = time(NULL) + timeout; > + } > I think that it would be safer to not set finish_time if > conn->connect_timeout is NULL. I agree that your code works because > pqWaitTimed() will never complain on timeout reached if finish_time is -1. > That's for robustness sake. Done, but I'm not sure how this contributes to the robustness. I guess you were concerned just in case pqWaitTimed() returned 0 (timeout) even when it should not. Regards Takayuki Tsunakawa libpq-retry-connect-on-timeout_v2.patch Description: libpq-retry-connect-on-timeout_v2.patch -- 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] Re: [doc fix] PG10: wroing description on connect_timeout when multiple hosts are specified
Hello Robert, Tom, Michael, Thanks a lot for checking my patch. Sorry, let me check Michael's review comments and reply tomorrow. Regards Takayuki Tsunakawa -- 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] [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur
From: Michael Paquier [mailto:michael.paqu...@gmail.com] > On Fri, May 12, 2017 at 10:44 PM, Tom Lanewrote: > > I would not really expect that reconnection would retry after > > arbitrary failure cases. Should it retry for "wrong database name", for > instance? > > It's not hard to imagine that leading to very confusing behavior. > > I guess not as well. That would be tricky for the user to have a different > behavior depending on the error returned by the server, which is why the > current code is doing things right IMO. Now, the feature has been designed > similarly to JDBC with its parametrization, so it could be surprising for > users to get a different failure handling compared to that. Not saying that > JDBC is doing it wrong, but libpq does nothing wrong either. I didn't intend to make the user have a different behavior depending on the error returned by the server. I meant attempting connection to alternative hosts when the server returned an error. I thought the new libpq feature tries to connect to other hosts when a connection attempt fails, where the "connection" is the *database connection* (user's perspective), not the *socket connection* (PG developer's perspective). I think PgJDBC meets the user's desire better -- "Please connect to some host for better HA if a database server is unavailable for some reason." By the way, could you elaborate what problem could occur if my solution is applied? (it doesn't seem easy for me to imagine...) FYI, as below, the case Tom picked up didn't raise an issue: [libpq] $ psql -h localhost,localhost -p 5450,5451 -d aaa psql: FATAL: database "aaa" does not exist $ [JDBC] $ java org.hsqldb.cmdline.SqlTool postgres SqlTool v. 3481. 2017-05-15T10:23:55.991+0900 SEVERE Connection error: org.postgresql.util.PSQLException: FATAL: database "aaa" does not exist Location: File: postinit.c, Routine: InitPostgres, Line: 846 Server SQLState: 3D000 at org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2412) at org.postgresql.core.v3.QueryExecutorImpl.readStartupMessages(QueryExecutorImpl.java:2538) at org.postgresql.core.v3.QueryExecutorImpl.(QueryExecutorImpl.java:122) at org.postgresql.core.v3.ConnectionFactoryImpl.openConnectionImpl(ConnectionFactoryImpl.java:227) at org.postgresql.core.ConnectionFactory.openConnection(ConnectionFactory.java:49) at org.postgresql.jdbc.PgConnection.(PgConnection.java:194) at org.postgresql.Driver.makeConnection(Driver.java:431) at org.postgresql.Driver.connect(Driver.java:247) at java.sql.DriverManager.getConnection(DriverManager.java:664) at java.sql.DriverManager.getConnection(DriverManager.java:247) at org.hsqldb.lib.RCData.getConnection(Unknown Source) at org.hsqldb.cmdline.SqlTool.objectMain(Unknown Source) at org.hsqldb.cmdline.SqlTool.main(Unknown Source) Failed to get a connection to 'jdbc:postgresql://localhost:5450,localhost:5451/aaa' as user "tunakawa". Cause: FATAL: database "aaa" does not exist Location: File: postinit.c, Routine: InitPostgres, Line: 846 Server SQLState: 3D000 $ Regards Takayuki Tsunakawa -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [doc fix] PG10: wroing description on connect_timeout when multiple hosts are specified
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tsunakawa,> Takayuki > Instead, I think we should fix the program to match the documented behavior. > Otherwise, if the first database machine is down, libpq might wait for about > 2 hours (depending on the OS's TCP keepalive setting), during which it tims > out after connect_timeout and does not attempt to connect to other hosts. > > I'll add this item in the PostgreSQL 10 Open Items. Please use the attached patch to fix the problem. I confirmed the success as follows: $ add "post_auth_delay = 10" in postgresql.conf on host1 $ start database servers on host1 and host2 $ psql -h host1,host2 -p 5432,5433 -d "dbname=postgres connect_timeout=3" (psql connected to host2 after 3 seconds, which I checked with \conninfo) Regards Takayuki Tsunakawa diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index eb5aaf7098..d02e5201fa 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -1720,6 +1720,8 @@ connectDBComplete(PGconn *conn) { PostgresPollingStatusType flag = PGRES_POLLING_WRITING; time_t finish_time = ((time_t) -1); + int ret = 0; + int timeout = 0; if (conn == NULL || conn->status == CONNECTION_BAD) return 0; @@ -1729,8 +1731,7 @@ connectDBComplete(PGconn *conn) */ if (conn->connect_timeout != NULL) { - int timeout = atoi(conn->connect_timeout); - + timeout = atoi(conn->connect_timeout); if (timeout > 0) { /* @@ -1761,7 +1762,8 @@ connectDBComplete(PGconn *conn) return 1; /* success! */ case PGRES_POLLING_READING: - if (pqWaitTimed(1, 0, conn, finish_time)) + ret = pqWaitTimed(1, 0, conn, finish_time); + if (ret == -1) { conn->status = CONNECTION_BAD; return 0; @@ -1769,7 +1771,8 @@ connectDBComplete(PGconn *conn) break; case PGRES_POLLING_WRITING: - if (pqWaitTimed(0, 1, conn, finish_time)) + ret = pqWaitTimed(0, 1, conn, finish_time); + if (ret == -1) { conn->status = CONNECTION_BAD; return 0; @@ -1782,6 +1785,22 @@ connectDBComplete(PGconn *conn) return 0; } + if (ret == 1) /* connect_timeout elapsed */ + { + /* If there are no more hosts, return (the error message is already set) */ + if (++conn->whichhost >= conn->nconnhost) + { + conn->whichhost = 0; + conn->status = CONNECTION_BAD; + return 0; + } + /* Attempt connection to the next host, starting the connect_timeout timer */ + pqDropConnection(conn, true); + conn->addr_cur = conn->connhost[conn->whichhost].addrlist; + conn->status = CONNECTION_NEEDED; + finish_time = time(NULL) + timeout; + } + /* * Now try to advance the state machine. */ diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index 756c6d7779..1d6ea93a0a 100644 --- a/src/interfaces/libpq/fe-misc.c +++ b/src/interfaces/libpq/fe-misc.c @@ -991,11 +991,9 @@ pqWait(int forRead, int forWrite, PGconn *conn) /* * pqWaitTimed: wait, but not past finish_time. * - * If finish_time is exceeded then we return failure (EOF). This is like - * the response for a kernel exception because we don't want the caller - * to try to read/write in that case. - * * finish_time = ((time_t) -1) disables the wait limit. + * + * Returns -1 on failure, 0 if the socket is readable/writable, 1 if it timed out. */ int pqWaitTimed(int forRead, int forWrite, PGconn *conn, time_t finish_time) @@ -1005,13 +1003,13 @@ pqWaitTimed(int forRead, int forWrite, PGconn *conn, time_t finish_time) result = pqSocketCheck(conn, forRead, forWrite, finish_time); if (result < 0) - return EOF; /* errorMessage is already set */ + return -1; /* errorMe
[HACKERS] Re: [doc fix] PG10: wroing description on connect_timeout when multiple hosts are specified
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tsunakawa, > Takayuki > I found a wrong sentence here in the doc. I'm sorry, this is what I asked > you to add... > > https://www.postgresql.org/docs/devel/static/libpq-connect.html#libpq- > paramkeywords > > connect_timeout > Maximum wait for connection, in seconds (write as a decimal integer string). > Zero or not specified means wait indefinitely. It is not recommended to > use a timeout of less than 2 seconds. This timeout applies separately to > each connection attempt. For example, if you specify two hosts and both > of them are unreachable, and connect_timeout is 5, the total time spent > waiting for a connection might be up to 10 seconds. > > > The program behavior is that libpq times out after connect_timeout seconds > regardless of how many hosts are specified. I confirmed it like this: > > $ export PGOPTIONS="-c post_auth_delay=30" > $ psql -d "dbname=postgres connect_timeout=5" -h localhost,localhost -p > 5432,5433 > (psql erros out after 5 seconds) > > Could you fix the doc with something like this? > > "This timeout applies across all the connection attempts. For example, if > you specify two hosts and both of them are unreachable, and connect_timeout > is 5, the total time spent waiting for a connection is up to 5 seconds." > > Should we also change the minimum "2 seconds" part to be longer, according > to the number of hosts? Instead, I think we should fix the program to match the documented behavior. Otherwise, if the first database machine is down, libpq might wait for about 2 hours (depending on the OS's TCP keepalive setting), during which it tims out after connect_timeout and does not attempt to connect to other hosts. I'll add this item in the PostgreSQL 10 Open Items. Regards Takayuki Tsunakawa -- 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] [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur
From: Michael Paquier [mailto:michael.paqu...@gmail.com] > It seems to me that the feature is behaving as wanted. Or in short attempt > to connect to the next host only if a connection cannot be established. > If there is a failure once the exchange with the server has begun, just > consider it as a hard failure. This is an important property for > authentication and SSL connection failures actually. But PgJDBC behaves as expected -- attempt another connection to other hosts (and succeed). I believe that's what users would naturally expect. The current libpq implementation handles only the socket-level connect failure. Regards Takayuki Tsunakawa -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur
Hello, I found a problem with libpq connection failover. When libpq cannot connect to earlier hosts in the host list, it doesn't try to connect to other hosts. For example, when you specify a wrong port that some non-postgres program is using, or some non-postgres program is using PG's port unexpectedly, you get an error like this: $ psql -h localhost -p 23 psql: received invalid response to SSL negotiation: $ psql -h localhost -p 23 -d "sslmode=disable" psql: expected authentication request from server, but received Likewise, when the first host has already reached max_connections, libpq doesn't attempt the connection aginst later hosts. The attached patch fixes this. I'll add this item in the PostgreSQL 10 Open Items. Regards Takayuki Tsunakawa libpq-reconnect-on-error.patch Description: libpq-reconnect-on-error.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [doc fix] PG10: wroing description on connect_timeout when multiple hosts are specified
Hello, Robert I found a wrong sentence here in the doc. I'm sorry, this is what I asked you to add... https://www.postgresql.org/docs/devel/static/libpq-connect.html#libpq-paramkeywords connect_timeout Maximum wait for connection, in seconds (write as a decimal integer string). Zero or not specified means wait indefinitely. It is not recommended to use a timeout of less than 2 seconds. This timeout applies separately to each connection attempt. For example, if you specify two hosts and both of them are unreachable, and connect_timeout is 5, the total time spent waiting for a connection might be up to 10 seconds. The program behavior is that libpq times out after connect_timeout seconds regardless of how many hosts are specified. I confirmed it like this: $ export PGOPTIONS="-c post_auth_delay=30" $ psql -d "dbname=postgres connect_timeout=5" -h localhost,localhost -p 5432,5433 (psql erros out after 5 seconds) Could you fix the doc with something like this? "This timeout applies across all the connection attempts. For example, if you specify two hosts and both of them are unreachable, and connect_timeout is 5, the total time spent waiting for a connection is up to 5 seconds." Should we also change the minimum "2 seconds" part to be longer, according to the number of hosts? Regards Takayuki Tsunakawa -- 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 v1] Add and report the new "in_hot_standby" GUC pseudo-variable.
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Craig Ringer > On 13 April 2017 at 14:59, Tsunakawa, Takayuki > <tsunakawa.ta...@jp.fujitsu.com> wrote: > > > 2. Make transaction_read_only GUC_REPORT This is to avoid the added > > round-trip by SHOW command. It also benefits client apps that want to > know when the server gets promoted? And this may simplify the libpq code. > > I don't understand yet why we need to provide this feature for older servers > by using SHOW. Those who are already using <= 9.6 in production completed > the system or application, and their business is running. Why would they > want to just replace libpq and use this feature? > > I think "transaction_read_only" is a bit confusing for something we're > expecting to change under us. > > To me, a "read only" xact is one created with > > BEGIN READ ONLY TRANSACTION; > > which I would not expect to become read/write under me, since I > explicitly asked for read-only. > > It's more like "session read only" that we're interested in IMO. Are you suggest thating we provide a GUC_REPORT read-only variable "session_read_only" and the libpq should use it? Anyway, I added this item in the PostgreSQL 10 Open Items page under "Design Decisions to Recheck Mid-Beta". I'm willing to submit a patch for PG10. Regards Takayuki Tsunakawa -- 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] [PostgreSQL 10] default of hot_standby should be "on"?
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Masahiko Sawada > The idea of changing the default value seems good to me but I'm not sure > it's good idea to change the default value now under the circumstances where > we're focus on stabilization. > Also we should update the document as well. > We can consider like this: the OP found a usability problem as a result of PG 10 development, so we will fix it as a stabilization work. Regards Takayuki Tsunakawa -- 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] PG 10 release notes
From: 'Bruce Momjian' [mailto:br...@momjian.us] > > I forgot to point out one thing. > > > > Allow libpq to connect to multiple specified host names (Robert Haas) > > libpq will connect with the first responsive host name. > > > > According to the following CF entry and my memory, > > > > https://commitfest.postgresql.org/12/879/ > > > > Authors > > mithun cy (mithun.cy) > > > > Reviewers > > Gerdan Santos (gerdan), Takayuki Tsunakawa (maumau)Remove from > > reviewers > > > > Committer > > Robert Haas (rhaas) > > > > The author should probably be "(Mithun Cy, Robert Haas, Victor Wagner)" > > I based the release note text on this commit: > > commit 274bb2b3857cc987cfa21d14775cae9b0dababa5 > Author: Robert Haas> Date: Thu Nov 3 09:25:20 2016 -0400 > > libpq: Allow connection strings and URIs to specify multiple > hosts. > > It's also possible to specify a separate port for each host. > > Previously, we'd loop over every address returned by looking > up the > host name; now, we'll try every address for every host name. > > Patch by me. Victor Wagner wrote an earlier patch for this > feature, > which I read, but I didn't use any of his code. Review by Mithun > Cy. > > Was it wrong? Oh, that commit. I got it. Regards Takayuki Tsunakawa -- 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] Cached plans and statement generalization
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Konstantin > Knizhnik > Well, first of all I want to share results I already get: pgbench with default > parameters, scale 10 and one connection: > > So autoprepare is as efficient as explicit prepare and can increase > performance almost two times. This sounds great. BTW, when are the autoprepared statements destroyed? Are you considering some upper limit on the number of prepared statements? Regards Takayuki Tsunakawa -- 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] PG 10 release notes
Hello, Bruce I forgot to point out one thing. Allow libpq to connect to multiple specified host names (Robert Haas) libpq will connect with the first responsive host name. According to the following CF entry and my memory, https://commitfest.postgresql.org/12/879/ Authors mithun cy (mithun.cy) Reviewers Gerdan Santos (gerdan), Takayuki Tsunakawa (maumau)Remove from reviewers Committer Robert Haas (rhaas) The author should probably be "(Mithun Cy, Robert Haas, Victor Wagner)" Regards Takayuki Tsunakawa -- 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] PG 10 release notes
Hello, Bruce > From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Bruce Momjian > I have committed the first draft of the Postgres 10 release notes. They > are current as of two days ago, and I will keep them current. Please give > me any feedback you have. Thanks for a nice release note. Below is what I noticed: (1) Support parallel bitmap heap scans (Dilip Kum) This item appears twice. (2) E.1.3.1.4. Optimizer Remove SCO and Unixware ports (Tom Lane) The section doesn't seem appropriate. Is OS-specific code related to the optimizer? (3) Remove documented restriction about using large shared buffers on Windows (Tsunakawa, Takayuki) Please change the name to "Takayuki Tsunakawa". (4) Have to_timestamp() and to_date() check check input values for validity (Artur Zakirov) "check" is repeated. (5) Use POSIX semaphores rather than SysV semaphores on Linux and FreeBSD (Tom Lane) This avoids some limits on SysV semaphores usage. These two lines are put in two different items. Regards Takayuki Tsunakawa -- 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 v1] Add and report the new "in_hot_standby" GUC pseudo-variable.
Hello, I didn't realize that my target_session_attrs naming proposal was committed. I didn't think half way it would be adopted, because the name is less intuitive than the original target_server_type, and is different from the PgJDBC's targetServerType. From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Simon Riggs > I agree if we introduce target_session_attrs it would be better to have > a complete feature in one release. > > It does seem quite strange to have > target_session_attrs=read-write > but not > target_session_attrs=read-only I totally agree. I'm a bit disappointed with and worried about the current situation. I could easily imagine that people around me would say a stern opinion on the specification... I think these are necessary in descending order of priority, if based on target_session_attrs: [PG10] 1. target_session_attrs=read-only This is mainly to connect to the standby. People will naturally expect that this is available, because PostgreSQL provides hot standby feature, and other DBMSs and even PgJDBC has the functionality. 2. Make transaction_read_only GUC_REPORT This is to avoid the added round-trip by SHOW command. It also benefits client apps that want to know when the server gets promoted? And this may simplify the libpq code. I don't understand yet why we need to provide this feature for older servers by using SHOW. Those who are already using <= 9.6 in production completed the system or application, and their business is running. Why would they want to just replace libpq and use this feature? [PG 11] 3. target_session_attrs=prefer-read-only This is mainly to prefer standbys, but allows to connect to the primary if no standby is available. Honestly, this is also required in PG 10 because PgJDBC already provides this by "preferSlave". From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas > On Tue, Apr 11, 2017 at 4:05 AM, Magnus Hagander> wrote: > > The part I'm talking about is the potential adjustment of the patch > > that's already committed. That's not a new feature, that's exactly the > > sort of thing we'd want to adjust before we get to release. Because > > once released we really can't change it. > > I don't really agree. I think if we go and install a GUC_REPORT GUC now, > we're much less likely to flush out the bugs in the 'show > transaction_read_only' mechanism. I'm sorry I couldn't get this part (maybe purely English nuance. Are you concerned about some bugs? We can't do anything if we fear of bugs. Is it OK instead to make transaction_read_only GUC_REPORT? > Also, now that I think about, the reason > why we settled on 'show transaction_read_only' against alternate queries > is because there's some ability for the DBA to make that return 'false' > rather than 'true' even when not in recovery, so that if for example you > are using logical replication rather than physical replication, you have > a way to make target_session_attrs=read-write still do something useful. > If you add an in_hot_standby GUC that's used instead, you lose that. Agreed. Again, is this satisfied by GUC_REPORTing transaction_read_only as well? > Now, we can decide what we want to do about that, but I don't see that a > change in this area *must* go into v10. Maybe the answer is that > target_session_attrs grows additional values like 'primary' and 'standby' > alongside 'read-write' (and Simon's suggested 'read-only'). > Or maybe we have another idea. But I don't really see the urgency of > whacking this around right this minute. There's nothing broken here; > there's just more stuff people would like to have. It can be added next > time around. But if completeness of the functionality is below people's expectations, it may unnecessarily compromise the reputation of PostgreSQL. Is there any chance to incorporate a patch into PG 10? May I add this as a PG 10 open item? Regards Takayuki Tsunakawa -- 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] Supporting huge pages on Windows
Hello, Magnus cc: Andres From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Magnus Hagander > I think what you'd need to do is enumerate what privileges the user has > *before* calling CreateRestrictedToken(), using GetTokenInformation(). > And then pass those into PrivilegesToDelete (except for > SeChangeNotifyPrivilege) in the call to CreateRestrictedToken(), instead > of using DISABLE_MAX_PRIVILEGE. (and add the privilege needed for huge pages > before you start that whole process -- that needs to be added in the token > used *before* we create the restricted one). > > At least that's my guess from reading the docs and trying to remember :) Oh, I got it now. Thanks. The revised patch is attached. The only modified file is pg_ctl.c. The patch worked as expected. It is regrettable that I could not make it in time for PG 10, but I'd appreciate it if you could review and commit this patch early in PG 11 while our memory is fresh. Thank you for your patience. I'll create an entry in the next CF soon. Regards Takayuki Tsunakawa win_large_pages_v13.patch Description: win_large_pages_v13.patch -- 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] Supporting huge pages on Windows
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Andres Freund > As I asked before, why can't we delete all privs and add the explicitly > needed once back (using AdjustTokenPrivileges)? I tried it with pg_ctl.c attached to an earlier mail today, i.e. delete all privs with CreateRestrictedToken(DISABLE_ALL_PRIVILEGE) and enable Lock Pages in Memory with AdjustTokenPrivileges(). But it didn't work; AdjustTokenPrivileges() failed to enable the priv. It's probably that CreateRestrictedToken() deletes (unassigns?) the privs from the access token, so subsequent AdjustTokenPrivileges() can no longer enable the priv. Regards Takayuki Tsunakawa -- 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] Statement timeout behavior in extended queries
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tatsuo Ishii > Since pgproto is a dumb protocol machine, it does not start a transaction > automatically (user needs to explicitly send a start transaction command > via either simple or extended query). In this particular case no explicit > transaction has started. > Then, the following sequence should have occurred. The test result is valid. # Execute statement which takes 2 seconds. 'P' "S1""SELECT pg_sleep(2)"0 -> start transaction T1 'B' "S2""S1"0 0 0 'P' "" "SET statement_timeout = '1s'" 0 'B' "" "" 0 0 0 'E' "" 0 # Execute statement which takes 2 seconds (statement timeout expected). 'E' "S2"0 -> timeout error occurred, T1 aborted # Issue Sync message 'S' -> rolled back T1, so statement_timeout reverted to 3s # Receive response from backend 'Y' # Execute statement which takes 2 seconds (statement timeout expected). 'P' "S3""SELECT pg_sleep(2)"0 -> start transaction T2 'B' "S2""S3"0 0 0 'E' "S2"0 # Issue Sync message 'S' -> committed T2 Regards Takayuki Tsunakawa -- 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] Statement timeout behavior in extended queries
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tatsuo Ishii > I have done tests using pgproto. One thing I noticed a strange behavior. > Below is an output of pgproto. The test first set the timeout to 3 seconds, > and parse/bind for "SELECT pg_sleep(2)" then set timeout to 1 second using > extended query. Subsequent Execute emits a statement timeout error as > expected, but next "SELECT pg_sleep(2)" > call using extended query does not emit a statement error. The test for > this is "007-timeout-twice". Attached is the test cases including this. What's the handling of transactions like in pgproto? I guess the first statement timeout error rolled back the effect of "SET statement_timeout = '1s'", and the timeout reverted to 3s or some other value. Regards Takayuki Tsunakawa -- 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] Statement timeout behavior in extended queries
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of 'Andres Freund' > Attached. I did not like that the previous patch had the timeout handling > duplicated in the individual functions, I instead centralized it into > start_xact_command(). Given that it already activated the timeout in the > most common cases, that seems to make more sense to me. In your version > we'd have called enable_statement_timeout() twice consecutively (which > behaviourally is harmless). > > What do you think? I've not really tested this with the extended protocol, > so I'd appreciate if you could rerun your test from the older thread. The patch looks good and cleaner. It looks like the code works as expected. As before, I ran one INSERT statement with PgJDBC, with gdb's breakpoints set on enable_timeout() and disable_timeout(). I confirmed that enable_timeout() is called just once at Parse message, and disable_timeout() is called just once at Execute message. I'd like to wait for Tatsuo-san's thorough testing with pgproto. Regards Takayuki Tsunakawa -- 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] Supporting huge pages on Windows
From: Craig Ringer [mailto:craig.rin...@2ndquadrant.com] > On 5 April 2017 at 10:37, Tsunakawa, Takayuki > <tsunakawa.ta...@jp.fujitsu.com> wrote: > > OTOH, I tried again to leave the DISABLE_MAX_PRIVILEGE as is and add Lock > Pages in Memory, using the attached pg_ctl.c. Please see > EnableLockPagesPrivilege() and its call site. But pg_ctl -w start fails > emitting the following message: > > That won't work. You'd have to pass 0 to the flags of CreateRestrictedToken > and instead supply a PrivilegesToDelete array. > You'd probably GetTokenInformation and AND with a mask of ones you wanted > to retain. Uh, that's inconvenient. We can't determine what privileges to delete, and we must be aware of new privileges added in the future version of Windows. Then, I have to say the last patch (v12) is the final answer. Regards Takayuki Tsunakawa -- 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] Supporting huge pages on Windows
From: Craig Ringer [mailto:craig.rin...@2ndquadrant.com] > TBH, anyone who cares about security and runs Win7 or Win2k8 or newer should > be using virtual service accounts and managed service accounts. > > https://technet.microsoft.com/en-us/library/dd548356 > > > Those are more like Unix service accounts. Notably they don't need a password, > getting rid of some of the management pain that led us to abandon the > 'postgres' system user on Windows. > > Now that older platforms are EoL and even the oldest that added this feature > are also near EoL or in extended maintenance, I think installers should > switch to these by default instead of using NETWORK SERVICE. > > Then the issue of priv dropping would be a lesser concern anyway. Good point! And I said earlier in this thread, I think managing privileges (adding/revoking privileges from the user account) is the DBA's or sysadmin's duty, and PG's removing all privileges feels overkill. OTOH, I tried again to leave the DISABLE_MAX_PRIVILEGE as is and add Lock Pages in Memory, using the attached pg_ctl.c. Please see EnableLockPagesPrivilege() and its call site. But pg_ctl -w start fails emitting the following message: error code 1300 waiting for server to startFATAL: could not enable "Lock pages in memory" privilege HINT: Assign "Lock pages in memory" privilege to the Windows user account which runs PostgreSQL. LOG: database system is shut down stopped waiting pg_ctl: could not start server Examine the log output. error code 1300 is ERROR_NOT_ALL_ASSIGNED, which means AdjustTokenPrivileges() cound not enable Lock Pages in Memory privilege. It seems that the privilege cannot be enabled once it was removed with CreateRestrictedToken(DISABLE_MAX_PRIVILEGE). Regards Takayuki Tsunakawa /*- * * pg_ctl --- start/stops/restarts the PostgreSQL server * * Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group * * src/bin/pg_ctl/pg_ctl.c * *- */ #ifdef WIN32 /* * Need this to get defines for restricted tokens and jobs. And it * has to be set before any header from the Win32 API is loaded. */ #define _WIN32_WINNT 0x0501 #endif #include "postgres_fe.h" #include "libpq-fe.h" #include "pqexpbuffer.h" #include #include #include #include #include #include #include #include #ifdef HAVE_SYS_RESOURCE_H #include #include #endif #include "getopt_long.h" #include "miscadmin.h" /* PID can be negative for standalone backend */ typedef long pgpid_t; typedef enum { SMART_MODE, FAST_MODE, IMMEDIATE_MODE } ShutdownMode; typedef enum { NO_COMMAND = 0, INIT_COMMAND, START_COMMAND, STOP_COMMAND, RESTART_COMMAND, RELOAD_COMMAND, STATUS_COMMAND, PROMOTE_COMMAND, KILL_COMMAND, REGISTER_COMMAND, UNREGISTER_COMMAND, RUN_AS_SERVICE_COMMAND } CtlCommand; #define DEFAULT_WAIT60 static bool do_wait = false; static bool del_service_rid = false; static bool wait_set = false; static int wait_seconds = DEFAULT_WAIT; static bool wait_seconds_arg = false; static bool silent_mode = false; static ShutdownMode shutdown_mode = FAST_MODE; static int sig = SIGINT; /* default */ static CtlCommand ctl_command = NO_COMMAND; static char *pg_data = NULL; static char *pg_config = NULL; static char *pgdata_opt = NULL; static char *post_opts = NULL; static const char *progname; static char *log_file = NULL; static char *exec_path = NULL; static char *event_source = NULL; static char *register_servicename = "PostgreSQL"; /* FIXME: + version ID? */ static char *register_username = NULL; static char *register_password = NULL; static char *argv0 = NULL; static bool allow_core_files = false; static time_t start_time; static char postopts_file[MAXPGPATH]; static char version_file[MAXPGPATH]; static char pid_file[MAXPGPATH]; static char backup_file[MAXPGPATH]; static char recovery_file[MAXPGPATH]; static char promote_file[MAXPGPATH]; #ifdef WIN32 static DWORD pgctl_start_type = SERVICE_AUTO_START; static SERVICE_STATUS status; static SERVICE_STATUS_HANDLE hStatus = (SERVICE_STATUS_HANDLE) 0; static HANDLE shutdownHandles[2]; static pid_t postmasterPID = -1; #define shutdownEvent shutdownHandles[0] #define postmasterProcess shutdownHandles[1] #endif static void write_stderr(const char *fmt,...) pg_attribute_printf(1, 2); static void do_advice(void); static void do_help(void); static void set_mode(char *modeopt); static void set_sig(char *signame); static void do_init(void); static void do_start(void); static void do_stop(void); static void do_restart(void); static void do_reload(void); static void do_status(void); static void do_promote(void); static void do_kill(pgpid_t pid); static void print_msg(const char *msg); static void
Re: [HACKERS] Statement timeout behavior in extended queries
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tatsuo Ishii > Hmm. IMO, that could happen even with the current statement timeout > implementation as well. > > Or we could start/stop the timeout in exec_execute_message() only. This > could avoid the problem above. Also this is more consistent with > log_duration/log_min_duration_statement behavior than now. I think it's better to include Parse and Bind as now. Parse or Bind could take long time when the table has many partitions, the query is complex and/or very long, some DDL statement is running against a target table, or the system load is high. Firing statement timeout during or after many Parses is not a problem, because the first Parse started running some statement and it's not finished. Plus, Andres confirmed that major client drivers don't use such a pattern. There may be an odd behavior purely from the perspective of E.Q.P, that's a compromise, which Andres meant by "not perfect but." Regards Takayuki Tsunakawa -- 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] Statement timeout behavior in extended queries
From: Andres Freund [mailto:and...@anarazel.de] > Looks to me like npgsql doesn't do that either. None of libpq, pgjdbs and > npgsql doing it seems like some evidence that it's ok. And psqlODBC now uses always libpq. Now time for final decision? Regards Takayuki Tsunakawa -- 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] Statement timeout behavior in extended queries
From: Andres Freund [mailto:and...@anarazel.de] Given the concern raised in > http://archives.postgresql.org/message-id/12207.1491228316%40sss.pgh.p > a.us > I don't see this being ready for committer. If what Tatsuo-san said to Tom is correct (i.e. each Parse/Bind/Execute starts and stops the timer), then it's a concern and the patch should not be ready for committer. However, the current patch is not like that -- it seems to do what others in this thread are expecting. Regards Takayuki Tsunakawa -- 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] Statement timeout behavior in extended queries
From: Tatsuo Ishii [mailto:is...@sraoss.co.jp] > It's too late. Someone has already moved the patch to the next CF (for > PostgreSQL 11). Yes, but this patch will be necessary by the final release of PG 10 if the libpq batch/pipelining is committed in PG 10. I marked this as ready for committer in the next CF, so that some committer can pick up this patch and consider putting it in PG 10. If you decide to modify the patch, please change the status. Regards Takayuki Tsunakawa -- 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] Supporting huge pages on Windows
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Andres Freund > I don't think the errdetail is quite right - OpenProcessToken isn't really > a syscall, is it? But then it's a common pattern already in wind32_shmem.c... Yes, "Win32 API function" would be correct, but I followed the existing code... > > +errdetail("Failed system call was %s.", > > +"LookupPrivilegeValue"))); > > Other places in the file actually log the arguments to the function... The only place is CreateFileMapping. Other places (DuplicateHandle and MapViewOfFileEx) don't log arguments. I guess the original developer thought that size and name arguments to CreateFileMapping() might be useful for troubleshooting. > Wonder if we should quote "Lock Pages in Memory" or add dashes, to make > sure it's clear that that's the right? I saw several Microsoft pages, including a page someone introduced me here, and they didn't quote the user right. I'm comfortable with the current code, but I don't mind if the committer adds some quotation. > > + flProtect = PAGE_READWRITE | SEC_COMMIT | > SEC_LARGE_PAGES; > > Why don't we just add the relevant flag, instead of overwriting the previous > contents? I don't have strong opinion on this... > Uh - isn't that a behavioural change? Was this discussed? Yes, I described this in the first mail. Magnus asked about this later and I replied. Regards Takayuki Tsunakawa -- 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] Statement timeout behavior in extended queries
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tatsuo Ishii > Actually the statement timer is replaced with new statement timer value > in enable_statement_timeout(). The patch doesn't seem to behave like that. The Parse message calls start_xact_command() -> enable_statement_timeout() -> enable_timeout(), and set stmt_timer_started to true. Subsequent Bind and Execute messages call enable_statement_timeout(), but enable_statement_timeout() doesn't call enable_timeout() because stmt_timer_started is already true. > > It looks like the patch does the following. I think this is desirable, > because starting and stopping the timer for each message may be costly as > Tom said. > > Parse(statement1) > > start timer > > Bind(statement1, portal1) > > Execute(portal1) > > stop timer > > Sync I ran one INSERT statement using JDBC with log_min_messages = DEBUG3, and the server log shows what I said. DEBUG: parse : insert into a values(2) DEBUG: Set statement timeout LOG: duration: 0.431 ms parse : insert into a values(2) DEBUG: bind to LOG: duration: 0.127 ms bind : insert into a values(2) DEBUG: Disable statement timeout LOG: duration: 0.184 ms execute : insert into a values(2) DEBUG: snapshot of 1+0 running transaction ids (lsn 0/163BF28 oldest xid 561 latest complete 560 next xid 562) > This doesn't work in general use cases. Following pattern appears frequently > in applications. > > Parse(statement1) > Bind(statement1, portal1) > Execute(portal1) > Bind(statement1, portal1) > Execute(portal1) > : > : > Sync It works. The first Parse-Bind-Execute is measured as one unit, then subsequent Bind-Execute pairs are measured as other units. That's because each Execute ends the statement_timeout timer and the Bind message starts it again. I think this is desirable, so the current patch looks good. May I mark this as ready for committer? FYI, make check-world passed successfully. > Also what would happen if client just send a parse message and does nothing > after that? It's correct to trigger the statement timeout in this case, because the first SQL statement started (with the Parse message) and its execution is not finished (with Execute message.) Regards Takayuki Tsunakawa -- 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] Statement timeout behavior in extended queries
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tatsuo Ishii > No. Parse, bind and Execute message indivually stops and starts the > statement_timeout timer with the patch. Unless there's a lock conflict, > parse and bind will take very short time. So actually users could only > observe the timeout in execute message though. Where is the statement_timeout timer stopped when processing Parse and Bind messages? Do you mean the following sequence of operations are performed in this patch? Parse(statement1) start timer stop timer Bind(statement1, portal1) start timer stop timer Execute(portal1) start timer stop timer Sync It looks like the patch does the following. I think this is desirable, because starting and stopping the timer for each message may be costly as Tom said. Parse(statement1) start timer Bind(statement1, portal1) Execute(portal1) stop timer Sync > > (3) > > + /* > > +* Sanity check > > +*/ > > + if (!xact_started) > > + { > > + ereport(ERROR, > > + > (errcode(ERRCODE_INVALID_PARAMETER_VALUE), > > +errmsg("Transaction > must have been already started to set statement timeout"))); > > + } > > > > I think this fragment can be deleted, because enable/disable_timeout() > is used only at limited places in postgres.c, so I don't see the chance > of misuse. > > I'd suggest leave it as it is. Because it might be possible that the function > is used in different place in the future. Or at least we should document > the pre-condition as a comment. OK, I favor documenting to reduce code, but I don't have a strong opinion. I'd like to leave this to the committer. One thing to note is that you can remove { and } in the following code like other places. + if (!xact_started) + { + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), +errmsg("Transaction must have been already started to set statement timeout"))); + } Regards Takayuki Tsunakawa -- 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] wait event documentation
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas > Instead of continuing to repair this every time it gets broken, I propose > that we break this into one table that lists all the wait_event_type values > -- LWLock, Lock, BufferPin, Activity, Client, Extension, IPC, Timeout, and > IO -- and then a second table for each type that has multiple wait events > listing all of the wait events for that type. > > I also propose hoisting this out of section 28.2.3 - Viewing Statistics > - and making it a new toplevel section of chapter 28. So between the current > "28.3 Viewing Locks" and the current "28.4 Progress Reporting" we'd add > a new section "Wait Events" and link to that from 28.2.3. That would also > give us a place to include any general text that we want to have regarding > wait events, apart from the tables. +1 I'm visually impaired and using screen reader software which reads text by Synthetic speech. It was not easy for me to navigate a large table to find the first row of a particular wait event type. With your proposal, I'll be able to move among tables with a single key press ("T" and "Shift + T".) I'd also be happy about separating the section for wait events as you propose, because navigating in a long section is cumbersome. Regards Takayuki Tsunakawa -- 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] Statement timeout behavior in extended queries
Hello, I've reviewed the patch. My understanding is as follows. Please correct me if I'm wrong. * The difference is that the Execute message stops the statement_timeout timer, so that the execution of one statement doesn't shorten the timeout period of subsequent statements when they are run in batch followed by a single Sync message. * This patch is also necessary (or desirable) for the feature "Pipelining/batch mode support for libpq," which is being developed for PG 10. This patch enables correct timeout handling for each statement execution in a batch. Without this patch, the entire batch of statements is subject to statement_timeout. That's different from what the manual describes about statement_timeout. statement_timeout should measure execution of each statement. Below are what I found in the patch. (1) +static bool st_timeout = false; I think the variable name would better be stmt_timeout_enabled or stmt_timer_started, because other existing names use stmt to abbreviate statement, and thhis variable represents a state (see xact_started for example.) (2) +static void enable_statement_timeout(void) +{ +static void disable_statement_timeout(void) +{ "static void" and the remaining line should be on different lines, as other functions do. (3) + /* +* Sanity check +*/ + if (!xact_started) + { + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), +errmsg("Transaction must have been already started to set statement timeout"))); + } I think this fragment can be deleted, because enable/disable_timeout() is used only at limited places in postgres.c, so I don't see the chance of misuse. Regards Takayuki Tsunakawa -- 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] Supporting huge pages on Windows
From: Michael Paquier [mailto:michael.paqu...@gmail.com] > Amit is right, you had better use %zu as we do in all the other places of > the code dealing with Size variables that are printed. When compiling with > Visual Studio, Postgres falls back to the implementation of sprintf in > src/port/snprintf.c so that's not something to worry about. Thanks, fixed. Regards Takayuki Tsunakawa win_large_pages_v12.patch Description: win_large_pages_v12.patch -- 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] [bug fix] Savepoint-related statements terminates connection
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Alvaro Herrera > Ashutosh Bapat wrote: > > Please add this to the next commitfest. > > If this cannot be reproduced in 9.6, then it must be added to the Open Items > wiki page instead. I added this in next CF. Regards Takayuki Tsunakawa -- 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] Supporting huge pages on Windows
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Amit Kapila > You should use %zu instead of %llu to print Size type of variable. I did so at first, but it didn't work with Visual Studio 2010 at hand. It doesn't support %z which is added in C99. But "I" (capital i) instead of "ll" seems appropriate as the following page says. I chose this. https://en.wikipedia.org/wiki/Printf_format_string I For signed integer types, causes printf to expect ptrdiff_t-sized integer argument; for unsigned integer types, causes printf to expect size_t-sized integer argument. Commonly found in Win32/Win64 platforms. Regards Takayuki Tsunakawa win_large_pages_v11.patch Description: win_large_pages_v11.patch -- 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] Allow interrupts on waiting standby
From: Michael Paquier [mailto:michael.paqu...@gmail.com] > Oops, sorry for that, I quite mess up with this patch. The WaitLatch() call > should still have WL_POSTMASTER_DEATH so as it can leave earlier, but yes > I agree with your analysis that HandleStartupProcInterrupts() as this is > part of the redo work. Thank you, but did you remove WL_LATCH_SET from WaitLatch() intentionally? I understood you added it for startup process to respond quickly to events other than the postmaster death. Why don't we restore WL_LATCH_SET? I won't object to not adding the flag if there's a reason. I'll mark this as ready for committer when I see WL_LATCH_SET added (optional) and you have reported that you did the following test cases: * Startup process vanishes immediately after postmaster dies, while it is waiting for a recovery conflict to be resolved. * Startup process vanishes immediately after "pg_ctl stop -m fast", while it is waiting for a recovery conflict to be resolved. * Startup process resumes WAL application when max_standby_{archive | streaming}_delay is changed from the default -1 to a short period, e.g. 10s, and "pg_ctl reload" is performed, while it is waiting for a recovery conflict to be resolved. > > Did Simon's committed patch solve the problem as expected? > > Does not seem so, I'll let Simon comment on this matter... Agreed. I guess his patch for earlier releases should work if CHECK_FOR_INTERRUPTS() is replaced with HandleStartupProcInterrupts(). Regards Takayuki Tsunakawa -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [bug fix] Savepoint-related statements terminates connection
Hello, I found a trivial bug that terminates the connection. The attached patch fixes this. PROBLEM Savepoint-related statements in a multi-command query terminates the connection unexpectedly, as follows. $ psql -d postgres -c "SELECT 1; SAVEPOINT sp" FATAL: DefineSavepoint: unexpected state STARTED server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. connection to server was lost CAUSE 1. In exec_simple_query(), isTopLevel is set to false. isTopLevel = (list_length(parsetree_list) == 1); Then it is passed to PortalRun(). (void) PortalRun(portal, FETCH_ALL, isTopLevel, receiver, receiver, completionTag); 2. The isTopLevel flag is passed through ProcessUtility() to RequireTransactionChain(). RequireTransactionChain(isTopLevel, "SAVEPOINT"); 3. CheckTransactionChain() returns successfully here: /* * inside a function call? */ if (!isTopLevel) return; 4. Finally, unexpectedly called DefineSavepoint() reports invalid transaction block state. /* These cases are invalid. */ case TBLOCK_DEFAULT: case TBLOCK_STARTED: ... elog(FATAL, "DefineSavepoint: unexpected state %s", BlockStateAsString(s->blockState)); SOLUTION The manual page says "Savepoints can only be established when inside a transaction block." So just check for it. https://www.postgresql.org/docs/devel/static/sql-savepoint.html RESULT AFTER FIX $ psql -d postgres -c "SELECT 1; SAVEPOINT sp" ERROR: SAVEPOINT can only be used in transaction blocks Regards Takayuki Tsunakawa savept-in-multi-cmd.patch Description: savept-in-multi-cmd.patch -- 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] Supporting huge pages on Windows
From: Amit Kapila [mailto:amit.kapil...@gmail.com] > The latest patch looks good to me apart from one Debug message, so I have > marked it as Ready For Committer. Thank you so much! > + ereport(DEBUG1, > + (errmsg("disabling huge pages"))); > > I think this should be similar to what we display in sysv_shmem.c as below: > > elog(DEBUG1, "mmap(%zu) with MAP_HUGETLB failed, huge pages disabled: %m", > allocsize); I understood you suggested this to make the reason clear for disabling huge pages. OK, done as follows. + elog(DEBUG1, "CreateFileMapping(%llu) with SEC_LARGE_PAGES failed " +"due to insufficient large pages, huge pages disabled", +size); I hope this will be committed soon. Regards Takayuki Tsunakawa win_large_pages_v10.patch Description: win_large_pages_v10.patch -- 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] Allow interrupts on waiting standby
Hi Michael, Simon, From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Paquier > > Oh, I see. But how does the startup process respond quickly? It seems > that you need to call HandleStartupProcInterrupts() instead of > CHECK_FOR_INTERRUPTS(). But I'm not sure whether > HandleStartupProcInterrupts() can be called here. > > Bah. Of course you are right. We don't care about SetLatch() here as signals > are processed with a different code path than normal backends. So, I understood you agreed that CHECK_FOR_INTERRUPTS() here does nothing. But your patch still calls it: + /* An interrupt may have occurred while waiting */ + CHECK_FOR_INTERRUPTS(); I got confused because the problem is not defined in this thread. What problem does this patch address? These ones? * The startup process terminates as soon as postmaster dies. * pg_stat_activity does not show the wait event of startup process waiting for a recovery conflict resolution. My guess about why you decided to not call HandleStartupProcInterrupts() here is: * Respond to postmaster death here. * No need to reload config file here because there's no parameter to affect this conflict wait. But max_standby_{archive | streaming}_delay seems to affect the wait period. * No need to handle SIGTERM and exit here, because the startup process doesn't wait for a conflict resolution here when he can terminate. I think you can call HandleStartupProcInterrupts() here, instead of checking postmaster death. Did you perform tests? Did Simon's committed patch solve the problem as expected? Regards Takayuki Tsunakawa -- 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] Allow interrupts on waiting standby
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Paquier > > By the way, doesn't this wait event belong to IPC wait event type, because > the process is waiting for other conflicting processes to terminate the > conflict conditions? Did you choose Timeout type because > max_standby_{archive | streaming}_delay relates to this wait? I'm not > confident which is appropriate, but I'm afraid users can associate this > wait with a timeout. > > Actually I think that this event belongs to the timeout category, because > we wait until the timeout has finished, the latch being here to be sure > that the process is more responsive in case of a postmaster death. OK. I confirmed the doc is fixed. > > (2) standby.c > > Do we need to specify WL_LATCH_SET? Who can set the latch? Do the > backends who ended the conflict set the latch? > > This makes the process able to react on SIGHUP. That's useful for > responsiveness. Oh, I see. But how does the startup process respond quickly? It seems that you need to call HandleStartupProcInterrupts() instead of CHECK_FOR_INTERRUPTS(). But I'm not sure whether HandleStartupProcInterrupts() can be called here. > > (3) standby.c > > + if (rc & WL_LATCH_SET) > > + ResetLatch(MyLatch); > > + > > + /* emergency bailout if postmaster has died */ > > + if (rc & WL_POSTMASTER_DEATH) > > + proc_exit(1); > > > > I thought the child processes have to terminate as soon as postmaster > vanishes. So, it would be better for the order of the two if statements > above to be reversed. proc_exit() could be exit(), as some children do > in postmaster/*.c. > > OK, reversed this order. I think exit() instead of proc_exit() better represents what the code wants to do -- terminate the process ASAP without cleaning up. Many other background children do so. Regards Takayuki Tsunakawa -- 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] Allow interrupts on waiting standby
(4) standby.c > The latch is not reset when the wait timed out. The next WaitLatch() would > return immediately. Sorry, let me withdraw this. This is my misunderstanding. OTOH, when is the latch reset before the wait? Is there an assumption that MyLatch has been in reset state since it was initialized? Is it safe to use MyLatch here, not the special latch like something used in xlog.c? Regards Takayuki Tsunakawa -- 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] Allow interrupts on waiting standby
Hi, Michael, From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Paquier > What do you think about the updated version attached? I reviewed this patch. Here are some comments and questions: (1) monitoring.sgml The new row needs to be placed in the "Timeout" group. The current patch puts the row at the end of IO group. Please insert the new row according to the alphabetical order. In addition, "morerows" attribute of the following line appears to be increased. Timeout By the way, doesn't this wait event belong to IPC wait event type, because the process is waiting for other conflicting processes to terminate the conflict conditions? Did you choose Timeout type because max_standby_{archive | streaming}_delay relates to this wait? I'm not confident which is appropriate, but I'm afraid users can associate this wait with a timeout. (2) standby.c Do we need to specify WL_LATCH_SET? Who can set the latch? Do the backends who ended the conflict set the latch? (3) standby.c + if (rc & WL_LATCH_SET) + ResetLatch(MyLatch); + + /* emergency bailout if postmaster has died */ + if (rc & WL_POSTMASTER_DEATH) + proc_exit(1); I thought the child processes have to terminate as soon as postmaster vanishes. So, it would be better for the order of the two if statements above to be reversed. proc_exit() could be exit(), as some children do in postmaster/*.c. (4) standby.c The latch is not reset when the wait timed out. The next WaitLatch() would return immediately. Regards Takayuki Tsunakawa -- 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] Supporting huge pages on Windows
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of David Steele > It's not clear to me what state this patch should be in. Is there more > review that needs to be done or is it ready for a committer? I would most appreciate it if Magnus could review the code, because he gave me the most critical comment that the value of huge_page variable should not be changed on the fly, and I did so. I hope that will be the final review. Regards Takayuki Tsunakawa -- 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] Crash on promotion when recovery.conf is renamed
From: Michael Paquier [mailto:michael.paqu...@gmail.com] > Okay. I got the message, and I agree with what you say here. You are right > by the way, the error messages just use "two-phase file" and not "two-phase > STATE file", missed that previously. > -- Thanks, marked as ready for committer, as the code is good and Alexander reported the test success. Regards Takayuki Tsunakawa -- 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] Crash on promotion when recovery.conf is renamed
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Paquier > While I agree with that in general, we are taking about 2PC files that are > on disk in patch 0001, so I would think that in this case > ERRCODE_DATA_CORRUPTED is the most adapted (or > ERRCODE_TWOPHASE_CORRUPTED?). > > The other WARNING messages refer to stale files of already committed > transactions, which are not actually corrupted. What about in this case > having a ERRCODE_TWOPHASE_INVALID? > > Updated patches are attached, I did not change the WARNING portion though > as I am not sure what's the consensus on the matter. > I get the impression that DATA_CORRUPTED means the table data is corrupted, because there's an error code named INDEX_CORRUPTED. Anyway, I don't think this patch needs to attach an error code because: * Currently, other interlal files (not tables or indexes) seem to use INTERNAL_ERROR (XX000). For example, see ReadControlFile() in xlog.c and pgstat_read_statsfiles() in pgstat.c. * It doesn't seem that the user needs distinction. I don't object to providing a specific error code for this case, but if the patch needs a specific error code to be committed, I'd like to know how that's useful (e.g. how it affects the recovery action the user takes.) So, I'd like to mark the patch as ready for committer when ereport() and errmsg() are on separate lines and the message changes to "two-phase state file." Regards Takayuki Tsunakawa -- 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] On How To Shorten the Steep Learning Curve Towards PG Hacking...
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Kang Yuzhe > 1. Prepare Hands-on with PG internals > > > For example, a complete Hands-on with SELECT/INSERT SQL Standard PG > internals. The point is the experts can pick one fairly complex feature > and walk it from Parser to Executor in a hands-on manner explaining step > by step every technical detail. I sympathize with you. What level of detail do you have in mind? The query processing framework is described in the manual: Chapter 50. Overview of PostgreSQL Internals https://www.postgresql.org/docs/devel/static/overview.html More detailed source code analysis is provided for very old PostgreSQL 7.4, but I guess it's not much different now. The document is in Japanese only: http://ikubo.x0.com/PostgreSQL/pg_source.htm Are you thinking of something like this? MySQL Internals Manual https://dev.mysql.com/doc/internals/en/ Regards Takayuki Tsunakawa -- 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] Crash on promotion when recovery.conf is renamed
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tom Lane > "Tsunakawa, Takayuki" <tsunakawa.ta...@jp.fujitsu.com> writes: > > All other places in twophase.c and most places in other files put ereport() > and errmsg() on separate lines. I think it would be better to align with > surrounding code. > > > + ereport(FATAL, (errmsg("corrupted > two-phase file \"%s\"", > > Actually, the *real* problem with that coding is it lacks a SQLSTATE (errcode > call). The only places where it's acceptable to leave that out are for > internal "can't happen" cases, which this surely isn't. Oh, I overlooked it. But... > Yup. Just remember that the default is > XX000EERRCODE_INTERNAL_ERROR internal_error > > If that's not how you want the error case reported, you need an errcode() > call. > > We might need more ERRCODEs than are there now, if none of the existing > ones seem to fit these cases. There's already ERRCODE_DATA_CORRUPTED and > ERRCODE_INDEX_CORRUPTED; maybe we need ERRCODE_WAL_CORRUPTED, for > example? I'd be always happy if the error code is more specific, but maybe that would be a separate patch. WAL corruption message so far doesn't accompany a specific error code like this in xlog.c: /* * We only end up here without a message when XLogPageRead() * failed - in that case we already logged something. In * StandbyMode that only happens if we have been triggered, so we * shouldn't loop anymore in that case. */ if (errormsg) ereport(emode_for_corrupt_record(emode, RecPtr ? RecPtr : EndRecPtr), (errmsg_internal("%s", errormsg) /* already translated */ )); Regards Takayuki Tsunakawa -- 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] Potential data loss of 2PC files
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Paquier > Do you think that this qualifies as a bug fix for a backpatch? I would think > so, but I would not mind waiting for some dust to be on it before considering > applying that on back-branches. Thoughts from others? I think so, too. As change from rename() to durable_rename() was applied to all releases, maybe we can follow that. In addition, keeping code differences as few as possible would make it easier for committers to apply other bug fixes to all versions. Regards Takayuki Tsunakawa -- 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] Crash on promotion when recovery.conf is renamed
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Paquier > Moved to CF 2017-03. Both patches still apply. Sorry to be late for reviewing this, but done now. The patch applied, make check passed, and the code looks almost good. I could successfully perform a simple archive recovery. Finally, I broke the 2pc state file while the server is down, and could confirm that the server failed to start as expected, emitting a FATAL message. Worked nicely. Just two cosmetic points: (1) Other places use "two-phase state file", not "two-phase file". (2) All other places in twophase.c and most places in other files put ereport() and errmsg() on separate lines. I think it would be better to align with surrounding code. + ereport(FATAL, (errmsg("corrupted two-phase file \"%s\"", Regards Takayuki Tsunakawa -- 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] Do we create a new roadmap page for development?
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas > On Tue, Mar 21, 2017 at 2:36 AM, Tsunakawa, Takayuki > <tsunakawa.ta...@jp.fujitsu.com> wrote: > > Should I create a page for PostgreSQL 11 likewise? Or, do you want a > more stable page named "PostgreSQL Roadmap" instead of "PostgreSQL11 > Roadmap" and attach the target version to each feature as in "Feature X > (V11)", so that we can represent more longer-term goals? > > I think a page that's just called PostgreSQL Roadmap will get out of date > pretty quickly. Ultimately it'll end up like the TODO list, which is not > really a list of things TO DO. The advantage of the version-specific pages > is that old information kind of ages its way out of the system. Maybe so. I created a page for PostgreSQL 11 and add a link to our roadmap: https://wiki.postgresql.org/wiki/PostgreSQL11_Roadmap Please add your roadmaps when you can. > > And, why don't we add a link to the above roadmap page in the "Development > information" page? > > > > Development information > > https://wiki.postgresql.org/wiki/Development_information > > I'm sure nobody will object to you doing that. It's a wiki, and intended > to be edited. Thanks, done. Also, I moved the existing the link to "Development projects" from " Past PgCon Developer Meeting Notes" to the new header "Roadmaps and Projects", because the development projects page doesn't appear to fit the PGCON notes. Regards Takayuki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Do we create a new roadmap page for development?
Hello, I'd like to share our roadmap for PostgreSQL development, as other companies and individuals do in the following page. But this page is for PostgreSQL 10. PostgreSQL10 Roadmap https://wiki.postgresql.org/wiki/PostgreSQL10_Roadmap Should I create a page for PostgreSQL 11 likewise? Or, do you want a more stable page named "PostgreSQL Roadmap" instead of "PostgreSQL11 Roadmap" and attach the target version to each feature as in "Feature X (V11)", so that we can represent more longer-term goals? And, why don't we add a link to the above roadmap page in the "Development information" page? Development information https://wiki.postgresql.org/wiki/Development_information Regards Takayuki Tsunakawa -- 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: Make pg_stop_backup() archive wait optional
From: David Steele [mailto:da...@pgmasters.net] > Well, that's embarrassing. When I recreated the function to add defaults > I messed up the AS clause and did not pay attention to the results of the > regression tests, apparently. > > Attached is a new version rebased on 88e66d1. Catalog version bump has > also been omitted. I was late to reply because yesterday was a national holiday in Japan. I marked this as ready for committer. The patch applied cleanly and worked as expected. Regards Takayuki Tsunakawa -- 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: Make pg_stop_backup() archive wait optional
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tsunakawa, > Takayuki > I made this ready for committer. The patch applied except for catversion.h, > the patch content looks good, and the target test passed as follows: Sorry, I reverted this to waiting for author, because make check failed. src/test/regress/expected/opr_sanity.out seems to need revision because pg_stop_backup() was added. Regards Takayuki Tsunakawa -- 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] Potential data loss of 2PC files
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Ashutosh Bapat > The scope of this work has expanded, since last time I reviewed and marked > it as RFC. Right now I am busy with partition-wise joins and do not have > sufficient time to take a look at the expanded scope. > However, I can come back to this after partition-wise join, but that may > stretch to the end of the commitfest. Sorry. I marked this as ready for committer. The code looks good, make check passed, ran read/write pgbench for some period to cause checkpoint with WAL file removal, and pg_ctl stop. The checkpoint emitted the following message, and there were no message like "could not ..." that indicates a file unlink or directory sync failure. LOG: checkpoint complete: wrote 1835 buffers (11.2%); 0 transaction log file(s) added, 1 removed, 0 recycled; write=0.725 s, sync=0.002 s, total=0.746 s; s\ ync files=9, longest=0.002 s, average=0.000 s; distance=16381 kB, estimate=16381 kB Regards Takayuki Tsunakawa -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers