forked frompostgres/postgres
- Notifications
You must be signed in to change notification settings - Fork6
Commit5c97243
committed
Redesign handling of SIGTERM/control-C in parallel pg_dump/pg_restore.
Formerly, Unix builds of pg_dump/pg_restore would trap SIGINT and similarsignals and set a flag that was tested in various data-transfer loops.This was prone to errors of omission (cf commit3c8aa66); and even ifthe client-side response was prompt, we did nothing that would causelong-running SQL commands (e.g. CREATE INDEX) to terminate early.Also, the master process would effectively do nothing at all upon receiptof SIGINT; the only reason it seemed to work was that in typical scenariosthe signal would also be delivered to the child processes. We shouldsupport termination when a signal is delivered only to the master process,though.Windows builds had no console interrupt handler, so they would just fallover immediately at control-C, again leaving long-running SQL commands tofinish unmolested.To fix, remove the flag-checking approach altogether. Instead, allow theUnix signal handler to send a cancel request directly and then exit(1).In the master process, also have it forward the signal to the children.On Windows, add a console interrupt handler that behaves approximatelythe same. The main difference is that a single execution of the Windowshandler can send all the cancel requests since all the info is availablein one process, whereas on Unix each process sends a cancel only for itsown database connection.In passing, fix an old problem that DisconnectDatabase tends to send acancel request before exiting a parallel worker, even if nothing wentwrong. This is at least a waste of cycles, and could lead to unexpectedlog messages, or maybe even data loss if it happened in pg_restore (thoughin the current code the problem seems to affect only pg_dump). The causewas that after a COPY step, pg_dump was leaving libpq in PGASYNC_BUSYstate, causing PQtransactionStatus() to report PQTRANS_ACTIVE. That'snormally harmless because the next PQexec() will silently clear thePGASYNC_BUSY state; but in a parallel worker we might exit without anyadditional SQL commands after a COPY step. So add an extra PQgetResult()call after a COPY to allow libpq to return to PGASYNC_IDLE state.This is a bug fix, IMO, so back-patch to 9.3 where parallel dump/restorewere introduced.Thanks to Kyotaro Horiguchi for Windows testing and code suggestions.Original-Patch: <7005.1464657274@sss.pgh.pa.us>Discussion: <20160602.174941.256342236.horiguchi.kyotaro@lab.ntt.co.jp>1 parent0a1485f commit5c97243
File tree
8 files changed
+504
-168
lines changed- src/bin/pg_dump
8 files changed
+504
-168
lines changedLines changed: 0 additions & 9 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
183 | 183 | | |
184 | 184 | | |
185 | 185 | | |
186 | | - | |
187 | | - | |
188 | | - | |
189 | 186 | | |
190 | 187 | | |
191 | 188 | | |
| |||
355 | 352 | | |
356 | 353 | | |
357 | 354 | | |
358 | | - | |
359 | | - | |
360 | | - | |
361 | 355 | | |
362 | 356 | | |
363 | 357 | | |
| |||
418 | 412 | | |
419 | 413 | | |
420 | 414 | | |
421 | | - | |
422 | | - | |
423 | | - | |
424 | 415 | | |
425 | 416 | | |
426 | 417 | | |
| |||
0 commit comments
Comments
(0)