Re: [HACKERS] [Proposal] More Vacuum Statistics
Hi, Thank you for comments. and Sorry for my late response. pg_stat_vacuum view I understand it is not good to simply add more counters in pg_stat_*_tables. For now, I'd like to suggest an extension which can confirm vacuum statistics like pg_stat_statements. Similar feature has been already provided by pg_statsinfo package. But it is a full-stack package for PG-stats and it needs to redesign pg_log and design a repository database for introduce. And it is not a core-extension for PostgreSQL. (I don't intend to hate pg_statsinfo, I think this package is a very convinient tool) Everyone will be able to do more easily tuning of VACUUM. That's all I want. I'm still wondering whether these stats will really make the tuning any easier. What I do right now is looking at pg_stat_all_tables.n_deat_tup and if it exceeds some threshold, it's a sign that vacuum may need a bit of tuning. Sometimes it really requires tuning vacuum itself, but more often than not it's due to something else (a large bulk delete, autovacuum getting stuck on another table, ...). I don't see how the new stats would make this any easier. Can you give some examples on how the new stats might be used (and where the current stats are insufficient)? What use cases do you imagine for those stats? pg_stat_vacuum can keep histories of vacuum statistics for each tables/indices into shared memory.(They are not only last vacuum. This is already able to confirm using pg_stat_all_tables.) It makes easier analysis of vacuum histories because this view can sort or aggregate or filter. My use cases for those stats are following. - examine TRANSITION of vacuum execution time on any table (you can predict the future vacuum execution time) - examine EXECUTION INTERVAL of vacuum for each table (if too frequent, it should make vacuum-threshold tuning to up) - examine REST of dead-tuples just after vacuum (if dead-tuples remain, it may be due to any idle in transaction sessions) It might help differentiate the autovacuum activity from the rest of the system (e.g. there's a lot of I/O going on - how much of that is coming from autovacuum workers?). This would however require a more fine-grained reporting, because often the vacuums run for a very long time, especially on very large tables (which is exactly the case when this might be handy) - I just had a VACUUM that ran for 12 hours. These jobs should report the stats incrementally, not just once at the very end, because that makes it rather useless IMNSHO. +1 Certainly, VACUUM have often much execution time, I just had too. At present, we cannot predict when this vacuum finishes, what this vacuum is doing now, and whether this vacuum have any problem or not. Maybe, For DBAs, It might be better to show vacuum progress in pg_stat_activity. (if we'd do, add a free-style column like progress ?) This column might also be able to use for other long time commands like ANALYZE, CREATE/RE INDEX and COPY. To realize this feature, we certainly need to properly change pgstat_report_activity, use it more and add a new track-activity parameter. Regards, Anzai Naoya --- Naoya Anzai Engineering Department NEC Solution Inovetors, Ltd. E-Mail: nao-an...@xc.jp.nec.com --- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Comfortably check BackendPID with psql
Not a big fan of that abbreviation itself. What I'd wondered about instead - and actually had patched into my psql at some point - is adding an appropriate escape to psql's PROMPT. I think that'd serve your purpose as well? +3.14159; that would be hugely helpful when using gdb. You can get that today. In ~/.psqlrc: SELECT pg_catalog.pg_backend_pid() AS backend_pid \gset \set PROMPT1 '%m %:backend_pid: %/%R%# ' It doesn't update after \connect, but the overlap between my use of \connect and my use of debuggers is tiny. Thank you all! My hack is going to be much smoother. Regards, Naoya --- Naoya Anzai Engineering Department NEC Solution Inovetors, Ltd. E-Mail: nao-an...@xc.jp.nec.com --- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Comfortably check BackendPID with psql
Hi, hackers! This is a so tiny patch but I think it is very useful for hackers and DBAs. When we debug with psql, we frequently use SELECT pg_backend_pid();. This can change the input of the 24 characters to the only 4 characters! Image. -- naoya=# \bid Backend Process ID pid -- 1716 (1 row) --- How do you like it? Regards, Naoya --- Naoya Anzai Engineering Department NEC Solution Inovetors, Ltd. E-Mail: nao-an...@xc.jp.nec.com --- psql_show_backend_pid.patch Description: psql_show_backend_pid.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] Comfortably check BackendPID with psql
Hi, Andres, Marko Seems easier to set this in .psqlrc: oops! I've never noticed.. Thank you for your comment. Regards, Naoya --- Naoya Anzai Engineering Department NEC Solution Inovetors, Ltd. E-Mail: nao-an...@xc.jp.nec.com --- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [Proposal] More Vacuum Statistics
Thank you for quick feedback, and I'm sorry for slow response. All of your opinions were very helpful for me. I have confirmed Greg's Idea Timing events. http://www.postgresql.org/message-id/509300f7.5000...@2ndquadrant.com Greg said at first, Parsing log files for commonly needed performance data is no fun. Yes, I completely agree with him. That looks a nice idea but I don't know why this idea has not been commited yet. Anybody knows? I have reworked my idea since I heard dear hacker's opinions. pg_stat_vacuum view I understand it is not good to simply add more counters in pg_stat_*_tables. For now, I'd like to suggest an extension which can confirm vacuum statistics like pg_stat_statements. VACUUM is a most important feature in PostgreSQL, but a special view for vacuum does not exist. Don't you think the fact is inconvenience? At least, I am disgruntled with that we need to parse pg_log for tune VACUUM. My first design of pg_stat_vacuum view is following. (There are two views.) pg_stat_vacuum_table --- dbid schemaname relid relname elapsed page_removed page_remain page_skipped tuple_removed tuple_remain tuple_notremovable buffer_hit buffer_miss buffer_dirty avg_read avg_write vm_count vac_start vac_end is_autovacuum pg_stat_vacuum_index --- dbid shemaname relid indexrelid indexname elapsed num_index_tuples num_pages tuples_removed pages_deleted pages_free is_autovacuum At present, I think memory design of pg_stat_statements can divert into this feature.And I think this module needs to prepare following parameters like pg_stat_statements. pg_stat_vacuum.max(integer) pg_stat_vacuum.save(boolean) pg_stat_vacuum.excluded_dbnames(text) pg_stat_vacuum.excluded_schemas(text) pg_stat_vacuum.min_duration(integer) ... and so on. To implement this feature, I have to collect each vacuum-stats every lazy_vacuum_* and I need to embed a hook function point where needed. (probably last point of lazy_vacuum_rel). Do you hesitate to add the hook only for this function? Similar feature has been already provided by pg_statsinfo package. But it is a full-stack package for PG-stats and it needs to redesign pg_log and design a repository database for introduce. And it is not a core-extension for PostgreSQL. (I don't intend to hate pg_statsinfo, I think this package is a very convinient tool) Everyone will be able to do more easily tuning of VACUUM. That's all I want. Any comments are welcome! Best Regards, Naoya Anzai --- Naoya Anzai Engineering Department NEC Solution Inovetors, Ltd. E-Mail: nao-an...@xc.jp.nec.com --- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] why does txid_current() assign new transaction-id?
Thank you for comments. I understand your points. For only to read a current transaction-id, I know we just have to use txid_current_snapshot and that is a best way, but I feel a little bit hassle to explain columns of txid_current_snapshot for my supporting customers.. (Xmin is and Xmax is ) .. txid_current has had the behavior of assigning a new transaction XID when one is not assigned since its introduction. I don't think that it is wise to change it now the way you do as many applications surely rely on this assumption. Perhaps we could make the documentation clearer about those things though, changing the description of this function to get current transaction ID, and assign a new one if one is not assigned yet: http://www.postgresql.org/docs/devel/static/functions-info.html +1 I think that a description of txid_current is too rough and it might be confused some users. txid_current is a different operation depending on session situations, I feel if detail of txid_current is documented then it will be better. For example... Inside of the transaction-block(begin..end), returns a transaction-id used by this block. Outside of the transaction-block, returns a next transaction-id(but it is consumed by this function). Regards, Naoya --- Naoya Anzai Engineering Department NEC Solution Inovetors, Ltd. E-Mail: nao-an...@xc.jp.nec.com --- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Why does txid_current() assign new transaction-id?
Hi,hackers! I have a question about txid_current(). it is Why does txid_current() assign new transaction-id?. When we executes txid_current() outside of transaction block, it assigns new transaction-id. I guess it doesn't need to assign a new txid because txid_current() is just a read-only function. I found a replaceable function by walking through pg-code, that is GetStableLatestTransactionId(void). I attached a patch which changing just 1-line. Could you please check the code? Regards, Naoya --- Naoya Anzai Engineering Department NEC Solution Inovetors, Ltd. E-Mail: nao-an...@xc.jp.nec.com --- txid_current.patch Description: txid_current.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] why does txid_current() assign new transaction-id?
Hi,hackers! I have a question about txid_current(). it is Why does txid_current() assign new transaction-id?. When we executes txid_current() outside of transaction block, it assigns new transaction-id. I guess it doesn't need to assign a new txid because txid_current() is just a read-only function. I found a replaceable function by walking through pg-code, that is GetStableLatestTransactionId(void). I attached a patch which changing just 1-line. Could you please check the code? Regards, Naoya --- Naoya Anzai Engineering Department NEC Solution Inovetors, Ltd. E-Mail: nao-an...@xc.jp.nec.com --- txid_current.patch Description: txid_current.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] Table-level log_autovacuum_min_duration
Hi, Michael
I found that definition of VERBOSE and log_autovacuum is not
pretty match. For example, VERBOSE can output logs of
scanning indices and scanning detail of analyze, but
log_autovacuum can't output them.
Please see following sequences.
1. execute these queries.
DROP TABLE t1;
CREATE TABLE t1(id integer primary key,name text);
ALTER TABLE t1 SET (log_autovacuum_min_duration=0);
ALTER TABLE t1 ALTER COLUMN name SET STORAGE EXTERNAL;
INSERT INTO t1 SELECT GENERATE_SERIES(1,100),repeat('a',3000);
UPDATE t1 SET name='update';
2. after a while, output the following logs by autovacuum movement.
(For your convenience, I put the ### TYPE ### prefix on each logs.)
### VERBOSE ###INFO: vacuuming public.t1
### VERBOSE ###INFO: scanned index t1_pkey to remove 33 row versions
### VERBOSE ###DETAIL: CPU 0.00s/0.00u sec elapsed 0.00 sec.
### VERBOSE ###INFO: t1: removed 33 row versions in 1 pages
### VERBOSE ###DETAIL: CPU 0.00s/0.00u sec elapsed 0.00 sec.
### VERBOSE ###INFO: index t1_pkey now contains 100 row versions in 2 pages
### VERBOSE ###DETAIL: 33 index row versions were removed.
### VERBOSE ###0 index pages have been deleted, 0 are currently
reusable.
### VERBOSE ###CPU 0.00s/0.00u sec elapsed 0.00 sec.
### VERBOSE ###INFO: t1: found 100 removable, 100 nonremovable row
versions in 2 out of 2 pages
### VERBOSE ###DETAIL: 0 dead row versions cannot be removed yet.
### VERBOSE ###There were 0 unused item pointers.
### VERBOSE ###Skipped 0 pages due to buffer pins.
### VERBOSE ###0 pages are entirely empty.
### VERBOSE ###CPU 0.00s/0.00u sec elapsed 0.00 sec.
### LOG_AVAC ###LOG: automatic vacuum of table naoya.public.t1: index
scans: 1
### LOG_AVAC ###pages: 0 removed, 2 remain, 0 skipped due to pins
### LOG_AVAC ###tuples: 100 removed, 100 remain, 0 are dead but not
yet removable
### LOG_AVAC ###buffer usage: 47 hits, 4 misses, 4 dirtied
### LOG_AVAC ###avg read rate: 14.192 MB/s, avg write rate: 14.192 MB/s
### LOG_AVAC ###system usage: CPU 0.00s/0.00u sec elapsed 0.00 sec
### VERBOSE ###INFO: analyzing public.t1
### VERBOSE ###INFO: t1: scanned 2 of 2 pages, containing 100 live rows
and 0 dead rows; 100 rows in sample, 100 estimated total rows
### LOG_AVAC ###LOG: automatic analyze of table naoya.public.t1 system
usage: CPU 0.00s/0.00u sec elapsed 0.04 sec
### VERBOSE ###INFO: vacuuming pg_toast.pg_toast_72882
### VERBOSE ###INFO: scanned index pg_toast_72882_index to remove 200 row
versions
### VERBOSE ###DETAIL: CPU 0.00s/0.00u sec elapsed 0.00 sec.
### VERBOSE ###INFO: pg_toast_72882: removed 200 row versions in 50 pages
### VERBOSE ###DETAIL: CPU 0.00s/0.00u sec elapsed 0.00 sec.
### VERBOSE ###INFO: index pg_toast_72882_index now contains 0 row
versions in 2 pages
### VERBOSE ###DETAIL: 200 index row versions were removed.
### VERBOSE ###0 index pages have been deleted, 0 are currently
reusable.
### VERBOSE ###CPU 0.00s/0.00u sec elapsed 0.00 sec.
### VERBOSE ###INFO: pg_toast_72882: found 200 removable, 0 nonremovable
row versions in 50 out of 50 pages
### VERBOSE ###DETAIL: 0 dead row versions cannot be removed yet.
### VERBOSE ###There were 0 unused item pointers.
### VERBOSE ###Skipped 0 pages due to buffer pins.
### VERBOSE ###0 pages are entirely empty.
### VERBOSE ###CPU 0.00s/0.00u sec elapsed 0.00 sec.
### VERBOSE ###INFO: pg_toast_72882: truncated 50 to 0 pages
### VERBOSE ###DETAIL: CPU 0.00s/0.00u sec elapsed 0.03 sec.
### LOG_AVAC ###LOG: automatic vacuum of table
naoya.pg_toast.pg_toast_72882: index scans: 1
### LOG_AVAC ###pages: 50 removed, 0 remain, 0 skipped due to pins
### LOG_AVAC ###tuples: 200 removed, 0 remain, 0 are dead but not yet
removable
### LOG_AVAC ###buffer usage: 223 hits, 2 misses, 1 dirtied
### LOG_AVAC ###avg read rate: 0.457 MB/s, avg write rate: 0.228 MB/s
### LOG_AVAC ### system usage: CPU 0.00s/0.00u sec elapsed 0.03 sec
I think VACOPT_VERBOSE should not be easily replaced to log_min_duration=0.
And, in this v7 patch looks that VERBOSE log is always output
in INFO-Level when log_autovacuum_min_duration is set to 0.
Is this your point?
Regards,
---
Naoya
--
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] Table-level log_autovacuum_min_duration
You mean that ... Log_autovacuum_min_duration assumes a role of VACOPT_VERBOSE. Even if this parameter never use currently for manual vacuum, log_autovacuum_min_duration should be set zero(anytime output) when we executes VACUUM(or ANALYZE) VERBOSE. Is my understanding correct? If so,it sounds logical. Yup, that's my opinion. Now I don't know if people would mind to remove VACOPT_VERBOSE and replace the control it does by log_min_duration in VacuumStmt. At least both things are overlapping, and log_min_duration offers more options than the plain VACOPT_VERBOSE. OK. I agree with you. Please re-implement as you are thinking. If we can abolish VERBOSE option, I think it's ideal that we will prepare a new parameter like a log_min_duration_vacuum(and log_min_duration_analyze) which integrating VERBOSE feature and log_autovacuum_min_duration. What I think you are proposing here is a VERBOSE option that hypothetically gets activated if a manual VACUUM takes more than a certain amount specified by those parameters. I doubt this would be useful. In any case this is unrelated to this patch. I suspect manual vacuum often executes as semi-auto vacuum by job-scheduler, cron, etc in actual maintenance cases. Whether auto or manual, I think that's important to output their logs in the same mechanism. Sorry, I seem to have wandered from the subject. Naoya -- 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] Table-level log_autovacuum_min_duration
Hi, Michael-san An updated patch is attached, I'm sorry for confusing you. I think you don't have to implement this code to disable this feature with using value -2.Because this use case is a rare case, and there is a practical workaround using huge value like 2e9. (You suggested 2e9 to me, didn't you? :) ) So, please remove this code. Well, I see your point but this is not completely true: we could as well rely entirely on this parameter instead of VACOPT_VERBOSE to determine if autovacuum, a vacuum or an analyze are in verbose mode, and remove VACOPT_VERBOSE, but I can imagine people complaining if VACOPT_VERBOSE is removed. So let's set it up unconditionally to -1 in gram.y for now. However if people think that it is fine to remove VACOPT_VERBOSE, we could use exclusively this parameter in VacuumStmt. Or even add sanity checks at the top of vacuum() to ensure that VACOPT_VERBOSE is set only when log_min_duration is positive. Additional opinions on this matter are welcome. I understand your point at last. :) You mean that ... Log_autovacuum_min_duration assumes a role of VACOPT_VERBOSE. Even if this parameter never use currently for manual vacuum, log_autovacuum_min_duration should be set zero(anytime output) when we executes VACUUM(or ANALYZE) VERBOSE. Is my understanding correct? If so,it sounds logical. If we can abolish VERBOSE option, I think it's ideal that we will prepare a new parameter like a log_min_duration_vacuum(and log_min_duration_analyze) which integrating VERBOSE feature and log_autovacuum_min_duration. Regards, --- Naoya Anzai -- 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] Table-level log_autovacuum_min_duration
Thanks for your reply. Feature test (snip) I thought about using a special value like -2 to define the behavior you are mentioning here, aka with -2 disable custom value and GUC parameter but I don't think it is worth adding as that's an ugly 3 line of code of this type: if (log_min_duration == -2) enforce_log_min = -1; I disscussed about this use case with my co-workers, who said that code is not good, then we concluded that it was actually a rare case. If such a case sometimes happens in fact, I (or someone) will suggest a different way from this patch to handle this case. We know there is a practical workaround. :) Coding review I think description of log_autovacuum_min_duration in reloptions.c (line:215) should be like other parameters (match with guc.c). So it should be Sets the minimum execution time above which autovacuum actions will be logged. but not Log autovacuum execution for given threshold. What about that then? Minimum execution time above which autovacuum actions will be logged That's roughly match. For matching with guc.c, you might be better to add Sets the to that discription. Architecture review About the modification of gram.y. I think it is not good that log_min_duration is initialized to zeros in gram.y when FREEZE option is set. Because any VACUUM queries never use this parameter. I think log_min_duration always should be initialized to -1. Looking at this patch this morning, actually I think that my last patch as well as your suggestion are both wrong. To put it in short words, log_min_duration should be set only if VACOPT_VERBOSE is defined in query declaration. So I changed this portion of the patch this way. And I forgot to change VacuumStmt for the ANALYZE portion in gram.y... Patch updated attached. Sorry, I could not correctly express my opinion to you. I mean log_autovacuum_min_duration is used only by AutoVacuum, Any VACUUM queries (including VACUUM VERBOSE) never use this parameter. So, when someone executes Manual Vacuum, log_min_duration always should be initialized to -1. ANALYZE should also be the same. In other words, it is not necessary to initialize log_min_duration to zero when perform the VACUUM(or ANALYZE) VERBOSE. VERBOSE is provided only for changing the log level of Manual VACUUM from DEBUG2 to INFO. Regards, - Naoya. -- 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] Table-level log_autovacuum_min_duration
The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: tested, failed Spec compliant: tested, failed Documentation:tested, failed Hi, I'm Naoya Anzai. I've been working as a PostgreSQL Support Engineer for 6 years. I am a newcomer of reviewing, and My programming skill is not so high. But the following members also participate in this review. (We say Two heads are better than one. :)) Akira Kurosawa kurosawa-ak...@mxc.nes.nec.co.jp Taiki Kondo kondo-ta...@mxt.nes.nec.co.jp Huong Dangminh dangminh-hu...@mxm.nes.nec.co.jp So I believe reviewing patches is not difficult for us. This is a review of Table-level log_autovacuum_min_duration patch: http://www.postgresql.org/message-id/cab7npqtbqsbegvb8coh01k7argys9kbuv8dr+aqgonfvb8k...@mail.gmail.com Submission review The patch applies cleanly to HEAD, and it works fine on Fedora release 20. There is no regression test,but I think it is no problem because other parameter also is not tested. Usability review pg_dump/pg_restore support is OK. I think this feature is a nice idea and I also want this feature. Feature test I executed following commands after setting log_autovacuum_min_duration to 1000 in the GUC. (bar table is already created with no options.) CREATE TABLE foo ( ... ) WITH ( log_autovacuum_min_duration = 0 ); ALTER TABLE bar SET (log_autovacuum_min_duration = 0 ); Then, only in foo and bar table, autovacuum log was printed out even if elapsed time of autovacuum is lesser than 1000ms. This behavior was expected and there was no crash or failed assertion. So it looked good for me. But, I executed following command, in buzz table, autovacuum log was printed out if elapsed time is more than 1000ms. CREATE TABLE buzz ( ... ) WITH ( log_autovacuum_min_duration = -1 ); ^^ I expect autovacuum log is NOT printed out even if elapsed time is more than 1000ms in this case. My thought is wrong, isn't it? In my opinion, there is an use case that autovacuum log is always printed out in all tables excepting specified tables. I think there is a person who wants to use it like this case, but I (or he) can NOT use in this situation. How about your opinion? Performance review Not reviewed from this point of view. Coding review I think description of log_autovacuum_min_duration in reloptions.c (line:215) should be like other parameters (match with guc.c). So it should be Sets the minimum execution time above which autovacuum actions will be logged. but not Log autovacuum execution for given threshold. There is no new function which is defined in this patch, and modified contents are not related to OS-dependent contents, so I think it will work fine on Windows/BSD etc. Architecture review About the modification of gram.y. I think it is not good that log_min_duration is initialized to zeros in gram.y when FREEZE option is set. Because any VACUUM queries never use this parameter. I think log_min_duration always should be initialized to -1. Regards, Naoya Anzai (NEC Solution Innovators,Ltd.) The new status of this patch is: Waiting on Author -- 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] Table-level log_autovacuum_min_duration
The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: tested, failed Spec compliant: tested, failed Documentation:tested, failed I'm sorry, I just sent it by mistake. All of them have passed. --- Naoya Anzai -- 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] cancelling statement due to user request error occurs but the transaction has committed.
Hi, Well, the only other principled fix I can see is to add a new reponse along the lines of ERRORBUTITCOMMITTED, which does not seem attractive either, since all clients will have to be taught to understand it. +1 I think current specification hard to understand for many users. It is really good if PostgreSQL gave us a message such as a replication abort warning: ### WARNING: canceling wait for synchronous replication due to user request DETAIL: The transaction has already committed locally, but might not have been replicated to the standby. ### Regards, Naoya --- Naoya Anzai Engineering Department NEC Solution Inovetors, Ltd. E-Mail: anzai-na...@mxu.nes.nec.co.jp --- -- 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] cancelling statement due to user request error occurs but the transaction has committed.
Hi Amit, Thank you for your response. There can be similar observation if the server goes off (power outage or anything like) after committing transaction, client will receive connection broken, so he can misunderstand that as well. I think for such corner cases, client needs to reconfirm his action results with database before concluding anything. I see. Now, I understand that ProcessInterrupts Error (ProcDie, QueryCancel, ClientLost..) does not mean That query has been RollBacked. Regards, Naoya --- Naoya Anzai Engineering Department NEC Solution Inovetors, Ltd. E-Mail: anzai-na...@mxu.nes.nec.co.jp --- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] cancelling statement due to user request error occurs but the transaction has committed.
Hi All, When log_duration is true ( or log_min_duration_statement=0 ), If a transaction has internally been commited receives a SIGINT signal then a query cancellation error is output. For example, 1. A query like a TRUNCATE is removing bigger table files. 2. The session receives SIGINT signal. 3. Query cancellation error occurs. 4. But the query has commited. e.g.) --- naoya=# \d List of relations Schema | Name | Type | Owner +--+---+--- public | hoge | table | naoya (1 row) naoya=# set log_duration=on; SET naoya=# select count(*) from hoge; count 10 (1 row) naoya=# truncate hoge; Cancel request sent ERROR: canceling statement due to user request naoya=# select count(*) from hoge; count --- 0 (1 row) --- This is because ProcessInterrupts function is called by errfinish ( in query-duration ereport). I think this cancellation request must not interrupt the internal commited transaction. This is because clients may misunderstand the transaction has rollbacked. Now, I tried to fix the problem. --- postgresql-fe7337f/src/backend/utils/error/elog.c 2014-06-06 11:57:44.0 +0900 +++ postgresql-fe7337f.new/src/backend/utils/error/elog.c 2014-06-06 13:10:51.0 +0900 @@ -580,7 +580,8 @@ * can stop a query emitting tons of notice or warning messages, even if * it's in a loop that otherwise fails to check for interrupts. */ - CHECK_FOR_INTERRUPTS(); + if (IsTransactionState()) + CHECK_FOR_INTERRUPTS(); } Thereby, When ereport(non error level) calls and not in-transaction state, PostgreSQL never calls ProcessInterrupts function by errfinish. But I have a anxiety to fix errfinish function because errfinish is called in many many situations.. Could you please confirm it? Regards, Naoya --- Naoya Anzai Engineering Department NEC Solution Inovetors, Ltd. E-Mail: anzai-na...@mxu.nes.nec.co.jp --- postgresql-fe7337f_elog.patch Description: postgresql-fe7337f_elog.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] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application
Hi, Rajeev I tested the latest patch. My observation is: If we give relative data directory path while registering the service, then service start fails. But same works if the data directory is absolute path. Looks like an existing issue. May be we need to internally convert relative data path to absolute. Since the mentioned issue is an existing issue and not because of this patch. So can we take that as separate defect and fix. If so, then I can move this patch to ready for committer. I think so too. In boot by Service, CurrentDirectory seems to be C:/Windows/system32. So, you have to set a relative data directory path that the starting point to be C:/Windows/system32. Thanks and Regards, Kumar Rajeev Rastogi Regards, Naoya --- Naoya Anzai Engineering Department NEC Soft, Ltd. E-Mail: anzai-na...@mxu.nes.nec.co.jp --- -- 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 Service on Windows does not start. ~ is not a valid Win32 application
Hi Amit, I have uploaded your patch for next commit fest, hope you can support it if there is any feedback for your patch by reviewer/committer. Thanks! Okay, I will support you. Best Regards, Naoya Hi Naoya, On Thu, Oct 31, 2013 at 5:42 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Oct 31, 2013 at 1:44 AM, Asif Naeem anaeem...@gmail.com wrote: On Thu, Oct 31, 2013 at 10:17 AM, Amit Kapila amit.kapil...@gmail.com wrote: On Tue, Oct 29, 2013 at 12:46 PM, Naoya Anzai anzai-na...@mxu.nes.nec.co.jp wrote: Hi Sandeep I think, you should change the subject line to Unquoted service path containing space is vulnerable and can be exploited on Windows to get the attention.. :) Thank you for advice! I'll try to post to pgsql-bugs again. I could also reproduce this issue. The situation is very rare such that an exe with name same as first part of directory should exist in installation path. If one of the committers who is knowledgeable about Windows has time to apply this *before* the next CommitFest, that's obviously great. But the purpose of adding a link to the next CommitFest is to provide a backstop, so that we're not relying solely on someone to notice this email thread and pick it up, but instead have the patch as part of a list of patches needing review. I have uploaded your patch for next commit fest, hope you can support it if there is any feedback for your patch by reviewer/committer. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers Regards, Naoya --- Naoya Anzai Engineering Department NEC Soft, Ltd. E-Mail: anzai-na...@mxu.nes.nec.co.jp --- -- 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 Service on Windows does not start. ~ is not a valid Win32 application
Hi,Sandeep Thanks. Sorry, There was a mistake in what I said. I said Not only pg_ctl.exe but postgres.exe also have the same problem. but, to say it correctly, postgres.exe does not have the problem. Source that contains the problem is only pg_ctl.c. So, this is not an installer issue. Is this bug raised to the PostgreSQL community? If yes, you should submit the patch there. YES, I had submitted there already,But nobody has responded me yet. http://postgresql.1045698.n5.nabble.com/PostgreSQL-Service-on-Windows-does-not-start-td5774206.html Regards, Naoya So, this is not an installer issue. Is this bug raised to the PostgreSQL community? If yes, you should submit the patch there. On Tue, Oct 29, 2013 at 6:23 AM, Naoya Anzai anzai-na...@mxu.nes.nec.co.jp wrote: Hi, Asif Thank you for providing my patch (pg_ctl.c.patch) to Sandeep on my behalf. Good finding. I have attached another version of patch (pg_ctl.c_windows_vulnerability.patch) attached that has fewer lines of code changes, can you please take a look ?. Thanks. I think your patch is not sufficient to fix. Not only pg_ctl.exe but postgres.exe also have the same problem. Even if your patch is attached, A Path of postgres.exe passed to CreateRestrictedProcess is not enclosed in quotation.(See pgwin32_ServiceMain at pg_ctl.c) So, processing enclosed in quotation should do in both conditions. Regards, Naoya --- Naoya Anzai Engineering Department NEC Soft, Ltd. E-Mail: anzai-na...@mxu.nes.nec.co.jp --- Hi Sandeep, PFA Naoya's patch (pg_ctl.c.patch). Hi Naoya, Good finding. I have attached another version of patch (pg_ctl.c_windows_vulnerability.patch) attached that has fewer lines of code changes, can you please take a look ?. Thanks. Best Regards, Asif Naeem On Mon, Oct 28, 2013 at 4:46 PM, Sandeep Thakkar sandeep.thak...@enterprisedb.com wrote: Hi Dave We register the service using pg_ctl. When I manually executed the following on the command prompt, I saw that the service path of the registered service did not have the pg_ctl.exe path in quotes. May be it should be handled in the pg_ctl code. c:\Users\Sandeep Thakkar\Documentsc:\Program Files\PostgreSQL\9.3\bin\pg_ctl.e xe register -N pg-9.3 -U NT AUTHORITY\NetworkService -D c:\Program Files\P ostgreSQL\9.3\data -w Naoya, I could not find your patch here. Can you please share it again? On Mon, Oct 28, 2013 at 2:53 PM, Dave Page dp...@pgadmin.org wrote: Sandeep, can you look at this please? Thanks. On Mon, Oct 28, 2013 at 8:18 AM, Asif Naeem anaeem...@gmail.com wrote: It is related to windows unquoted service path vulnerability in the the installer that creates service path without quotes that make service.exe to look for undesirable path for executable. postgresql-9.3 service path : C:/Users/asif/Desktop/Program files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3 -D C:/Users/asif/Desktop/Program files/9.3/data -w service.exe C:\Users\asif\Desktop\Program NAME NOT FOUND C:\Users\asif\Desktop\Program.exe NAME NOT FOUND C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe ACCESS DENIED C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe ACCESS DENIED C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice NAME NOT FOUND C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice.exe NAME NOT FOUND C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N NAME NOT FOUND C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N.exe NAME NOT FOUND C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N postgresql-9.3 NAME INVALID C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N postgresql-9.3.exe NAME INVALID C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
Re: [HACKERS] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application
Hi Sandeep I think, you should change the subject line to Unquoted service path containing space is vulnerable and can be exploited on Windows to get the attention.. :) Thank you for advice! I'll try to post to pgsql-bugs again. BTW, in your case, the file Program should be an exe and not just any other file to exploit this vulnerability. Right? Yes, Program is a simple file I made. Best Regards, Naoya Hi Naoya I think, you should change the subject line to Unquoted service path containing space is vulnerable and can be exploited on Windows to get the attention.. :) BTW, in your case, the file Program should be an exe and not just any other file to exploit this vulnerability. Right? On Tue, Oct 29, 2013 at 11:34 AM, Naoya Anzai anzai-na...@mxu.nes.nec.co.jp wrote: Hi,Sandeep Thanks. Sorry, There was a mistake in what I said. I said Not only pg_ctl.exe but postgres.exe also have the same problem. but, to say it correctly, postgres.exe does not have the problem. Source that contains the problem is only pg_ctl.c. So, this is not an installer issue. Is this bug raised to the PostgreSQL community? If yes, you should submit the patch there. YES, I had submitted there already,But nobody has responded me yet. http://postgresql.1045698.n5.nabble.com/PostgreSQL-Service-on-Windows-does-not-start-td5774206.html Regards, Naoya So, this is not an installer issue. Is this bug raised to the PostgreSQL community? If yes, you should submit the patch there. On Tue, Oct 29, 2013 at 6:23 AM, Naoya Anzai anzai-na...@mxu.nes.nec.co.jp wrote: Hi, Asif Thank you for providing my patch (pg_ctl.c.patch) to Sandeep on my behalf. Good finding. I have attached another version of patch (pg_ctl.c_windows_vulnerability.patch) attached that has fewer lines of code changes, can you please take a look ?. Thanks. I think your patch is not sufficient to fix. Not only pg_ctl.exe but postgres.exe also have the same problem. Even if your patch is attached, A Path of postgres.exe passed to CreateRestrictedProcess is not enclosed in quotation.(See pgwin32_ServiceMain at pg_ctl.c) So, processing enclosed in quotation should do in both conditions. Regards, Naoya --- Naoya Anzai Engineering Department NEC Soft, Ltd. E-Mail: anzai-na...@mxu.nes.nec.co.jp --- Hi Sandeep, PFA Naoya's patch (pg_ctl.c.patch). Hi Naoya, Good finding. I have attached another version of patch (pg_ctl.c_windows_vulnerability.patch) attached that has fewer lines of code changes, can you please take a look ?. Thanks. Best Regards, Asif Naeem On Mon, Oct 28, 2013 at 4:46 PM, Sandeep Thakkar sandeep.thak...@enterprisedb.com wrote: Hi Dave We register the service using pg_ctl. When I manually executed the following on the command prompt, I saw that the service path of the registered service did not have the pg_ctl.exe path in quotes. May be it should be handled in the pg_ctl code. c:\Users\Sandeep Thakkar\Documentsc:\Program Files\PostgreSQL\9.3\bin\pg_ctl.e xe register -N pg-9.3 -U NT AUTHORITY\NetworkService -D c:\Program Files\P ostgreSQL\9.3\data -w Naoya, I could not find your patch here. Can you please share it again? On Mon, Oct 28, 2013 at 2:53 PM, Dave Page dp...@pgadmin.org wrote: Sandeep, can you look at this please? Thanks. On Mon, Oct 28, 2013 at 8:18 AM, Asif Naeem anaeem...@gmail.com wrote: It is related to windows unquoted service path vulnerability in the the installer that creates service path without quotes that make service.exe to look for undesirable path for executable. postgresql-9.3 service path : C:/Users/asif/Desktop/Program files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3 -D
Re: [HACKERS] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application
Hi, Asif. Thank you for response. C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start server starting This failure does not occur by the command line. PostgreSQL needs to start by Windows Service. Additionally,In this case, A file Program needs to be exist at C:\Users\asif\Desktop\, and postgres.exe needs to be exist at C:\Users\asif\Desktop\Program files\9.3\bin. C:\Users\asif\Desktop\Program files\9.3\bindir ... 4,435,456 postgres.exe 80,896 pg_ctl.exe ... C:\Users\asif\Desktoppdir ... 0 Program DIR Program files ... Regards, Naoya Hi Naoya, I am not able to reproduce the problem. Do you mean pg windows service installed by installer is not working or bin\pg_ctl binary is not accepting spaces in the patch ?. Following worked for me i.e. C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start server starting Can you please share the exact steps ?. Thanks. Regards, Muhammad Asif Naeem On Mon, Oct 28, 2013 at 10:26 AM, Naoya Anzai anzai-na...@mxu.nes.nec.co.jp wrote: Hi All, I have found a case that PostgreSQL Service does not start. When it happens, the following error appears. is not a valid Win32 application This failure occurs when the following conditions are true. 1. There is postgres.exe in any directory that contains a space, such as Program Files. e.g.) C:\Program Files\PostgreSQL\bin\postgres.exe 2. A file using the first white space-delimited tokens of that directory as the file name exists, and there is it in the same hierarchy. e.g.) C:\Program //file pg_ctl.exe as PostgreSQL Service creates a postgres process using an absolute path which indicates the location of postgres.exe,but the path is not enclosed in quotation. Therefore,if the above-mentioned conditions are true, CreateProcessAsUser(a Windows Function called by pg_ctl.exe) tries to create a process using the other file such as Program, so the service fails to start. Accordingly, I think that the command path should be enclosed in quotation. I created a patch to fix this failure, So could anyone confirm? Regards, Naoya --- Naoya Anzai Engineering Department NEC Soft, Ltd. E-Mail: anzai-na...@mxu.nes.nec.co.jp --- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers 以上、よろしくお願い致します。 NECソフト株式会社 PFシステム事業部 テーマソフト開発G 安西 直也 外線(03)5534-2353 内線(8)57-40364 Mail:NES-N2363 E-mail:anzai-na...@mxu.nes.nec.co.jp ≪本メールの取り扱い≫ ・区分：秘密 ・開示：必要最小限で可 ・持出：禁止 ・期限：無期限 ・用済後：廃棄 -- 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 Service on Windows does not start. ~ is not a valid Win32 application
Hi, Asif Thank you for providing my patch (pg_ctl.c.patch) to Sandeep on my behalf. Good finding. I have attached another version of patch (pg_ctl.c_windows_vulnerability.patch) attached that has fewer lines of code changes, can you please take a look ?. Thanks. I think your patch is not sufficient to fix. Not only pg_ctl.exe but postgres.exe also have the same problem. Even if your patch is attached, A Path of postgres.exe passed to CreateRestrictedProcess is not enclosed in quotation.(See pgwin32_ServiceMain at pg_ctl.c) So, processing enclosed in quotation should do in both conditions. Regards, Naoya --- Naoya Anzai Engineering Department NEC Soft, Ltd. E-Mail: anzai-na...@mxu.nes.nec.co.jp --- Hi Sandeep, PFA Naoya's patch (pg_ctl.c.patch). Hi Naoya, Good finding. I have attached another version of patch (pg_ctl.c_windows_vulnerability.patch) attached that has fewer lines of code changes, can you please take a look ?. Thanks. Best Regards, Asif Naeem On Mon, Oct 28, 2013 at 4:46 PM, Sandeep Thakkar sandeep.thak...@enterprisedb.com wrote: Hi Dave We register the service using pg_ctl. When I manually executed the following on the command prompt, I saw that the service path of the registered service did not have the pg_ctl.exe path in quotes. May be it should be handled in the pg_ctl code. c:\Users\Sandeep Thakkar\Documentsc:\Program Files\PostgreSQL\9.3\bin\pg_ctl.e xe register -N pg-9.3 -U NT AUTHORITY\NetworkService -D c:\Program Files\P ostgreSQL\9.3\data -w Naoya, I could not find your patch here. Can you please share it again? On Mon, Oct 28, 2013 at 2:53 PM, Dave Page dp...@pgadmin.org wrote: Sandeep, can you look at this please? Thanks. On Mon, Oct 28, 2013 at 8:18 AM, Asif Naeem anaeem...@gmail.com wrote: It is related to windows unquoted service path vulnerability in the the installer that creates service path without quotes that make service.exe to look for undesirable path for executable. postgresql-9.3 service path : C:/Users/asif/Desktop/Program files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3 -D C:/Users/asif/Desktop/Program files/9.3/data -w service.exe C:\Users\asif\Desktop\Program NAME NOT FOUND C:\Users\asif\Desktop\Program.exe NAME NOT FOUND C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe ACCESS DENIED C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe ACCESS DENIED C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice NAME NOT FOUND C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice.exe NAME NOT FOUND C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N NAME NOT FOUND C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N.exe NAME NOT FOUND C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N postgresql-9.3 NAME INVALID C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N postgresql-9.3.exe NAME INVALID C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N postgresql-9.3 -D NAME INVALID C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N postgresql-9.3 -D.exe NAME INVALID C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N postgresql-9.3 -D C:\Users\asif\Desktop\Program NAME INVALID C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N postgresql-9.3 -D C:\Users\asif\Desktop\Program.exe NAME INVALID C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data NAME INVALID C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data.exe NAME INVALID C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data -w NAME INVALID C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data -w.exe NAME INVALID Fix
[HACKERS] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application
Hi All, I have found a case that PostgreSQL Service does not start. When it happens, the following error appears. is not a valid Win32 application This failure occurs when the following conditions are true. 1. There is postgres.exe in any directory that contains a space, such as Program Files. e.g.) C:\Program Files\PostgreSQL\bin\postgres.exe 2. A file using the first white space-delimited tokens of that directory as the file name exists, and there is it in the same hierarchy. e.g.) C:\Program //file pg_ctl.exe as PostgreSQL Service creates a postgres process using an absolute path which indicates the location of postgres.exe,but the path is not enclosed in quotation. Therefore,if the above-mentioned conditions are true, CreateProcessAsUser(a Windows Function called by pg_ctl.exe) tries to create a process using the other file such as Program, so the service fails to start. Accordingly, I think that the command path should be enclosed in quotation. I created a patch to fix this failure, So could anyone confirm? Regards, Naoya --- Naoya Anzai Engineering Department NEC Soft, Ltd. E-Mail: anzai-na...@mxu.nes.nec.co.jp --- pg_ctl.c.patch Description: pg_ctl.c.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
