pgsql: Make snprintf.c follow the C99 standard for snprintf's result va
Make snprintf.c follow the C99 standard for snprintf's result value. C99 says that the result should be the number of bytes that would have been emitted given a large enough buffer, not the number we actually were able to put in the buffer. It's time to make our substitute implementation comply with that. Not doing so results in inefficiency in buffer-enlargement cases, and also poses a portability hazard for third-party code that might expect C99-compliant snprintf behavior within Postgres. In passing, remove useless tests for str == NULL; neither C99 nor predecessor standards ever allowed that except when count == 0, so I see no reason to expend cycles on making that a non-crash case for this implementation. Also, don't waste a byte in pg_vfprintf's local I/O buffer; this might have performance benefits by allowing aligned writes during flushbuffer calls. Discussion: https://postgr.es/m/[email protected] Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/805889d7d23fbecf5925443deb334aaeb6beaeb0 Modified Files -- src/port/snprintf.c | 94 +++-- 1 file changed, 63 insertions(+), 31 deletions(-)
pgsql: Fix pg_replication_slot example output
Fix pg_replication_slot example output The example output of pg_replication_slot is wrong. Correct it and make the output stable by explicitly listing columns to output. Author: Kyotaro Horiguchi Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/[email protected] Branch -- REL9_4_STABLE Details --- https://git.postgresql.org/pg/commitdiff/ae1011870a039f72efee6bacb02b7408af4714fc Modified Files -- doc/src/sgml/high-availability.sgml | 8 1 file changed, 4 insertions(+), 4 deletions(-)
pgsql: Clean up assorted misuses of snprintf()'s result value.
Clean up assorted misuses of snprintf()'s result value. Fix a small number of places that were testing the result of snprintf() but doing so incorrectly. The right test for buffer overrun, per C99, is "result >= bufsize" not "result > bufsize". Some places were also checking for failure with "result == -1", but the standard only says that a negative value is delivered on failure. (Note that this only makes these places correct if snprintf() delivers C99-compliant results. But at least now these places are consistent with all the other places where we assume that.) Also, make psql_start_test() and isolation_start_test() check for buffer overrun while constructing their shell commands. There seems like a higher risk of overrun, with more severe consequences, here than there is for the individual file paths that are made elsewhere in the same functions, so this seemed like a worthwhile change. Also fix guc.c's do_serialize() to initialize errno = 0 before calling vsnprintf. In principle, this should be unnecessary because vsnprintf should have set errno if it returns a failure indication ... but the other two places this coding pattern is cribbed from don't assume that, so let's be consistent. These errors are all very old, so back-patch as appropriate. I think that only the shell command overrun cases are even theoretically reachable in practice, but there's not much point in erroneous error checks. Discussion: https://postgr.es/m/[email protected] Branch -- REL9_5_STABLE Details --- https://git.postgresql.org/pg/commitdiff/c81062e8e12f4186a4d7d95e07541883f4e1d87e Modified Files -- src/backend/libpq/ip.c | 6 +++--- src/backend/postmaster/pgstat.c | 2 +- src/backend/utils/misc/guc.c| 2 ++ src/interfaces/ecpg/pgtypeslib/common.c | 2 +- src/port/getaddrinfo.c | 2 +- src/test/isolation/isolation_main.c | 24 ++-- src/test/regress/pg_regress.c | 2 +- src/test/regress/pg_regress_main.c | 26 +++--- 8 files changed, 46 insertions(+), 20 deletions(-)
pgsql: Clean up assorted misuses of snprintf()'s result value.
Clean up assorted misuses of snprintf()'s result value. Fix a small number of places that were testing the result of snprintf() but doing so incorrectly. The right test for buffer overrun, per C99, is "result >= bufsize" not "result > bufsize". Some places were also checking for failure with "result == -1", but the standard only says that a negative value is delivered on failure. (Note that this only makes these places correct if snprintf() delivers C99-compliant results. But at least now these places are consistent with all the other places where we assume that.) Also, make psql_start_test() and isolation_start_test() check for buffer overrun while constructing their shell commands. There seems like a higher risk of overrun, with more severe consequences, here than there is for the individual file paths that are made elsewhere in the same functions, so this seemed like a worthwhile change. Also fix guc.c's do_serialize() to initialize errno = 0 before calling vsnprintf. In principle, this should be unnecessary because vsnprintf should have set errno if it returns a failure indication ... but the other two places this coding pattern is cribbed from don't assume that, so let's be consistent. These errors are all very old, so back-patch as appropriate. I think that only the shell command overrun cases are even theoretically reachable in practice, but there's not much point in erroneous error checks. Discussion: https://postgr.es/m/[email protected] Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/6101bc2f459c3c5f4e8386083edb79443075b54e Modified Files -- src/backend/postmaster/pgstat.c | 2 +- src/backend/utils/misc/guc.c| 2 ++ src/common/ip.c | 6 +++--- src/interfaces/ecpg/pgtypeslib/common.c | 2 +- src/port/getaddrinfo.c | 2 +- src/test/isolation/isolation_main.c | 24 ++-- src/test/regress/pg_regress.c | 2 +- src/test/regress/pg_regress_main.c | 28 8 files changed, 47 insertions(+), 21 deletions(-)
pgsql: Clean up assorted misuses of snprintf()'s result value.
Clean up assorted misuses of snprintf()'s result value. Fix a small number of places that were testing the result of snprintf() but doing so incorrectly. The right test for buffer overrun, per C99, is "result >= bufsize" not "result > bufsize". Some places were also checking for failure with "result == -1", but the standard only says that a negative value is delivered on failure. (Note that this only makes these places correct if snprintf() delivers C99-compliant results. But at least now these places are consistent with all the other places where we assume that.) Also, make psql_start_test() and isolation_start_test() check for buffer overrun while constructing their shell commands. There seems like a higher risk of overrun, with more severe consequences, here than there is for the individual file paths that are made elsewhere in the same functions, so this seemed like a worthwhile change. Also fix guc.c's do_serialize() to initialize errno = 0 before calling vsnprintf. In principle, this should be unnecessary because vsnprintf should have set errno if it returns a failure indication ... but the other two places this coding pattern is cribbed from don't assume that, so let's be consistent. These errors are all very old, so back-patch as appropriate. I think that only the shell command overrun cases are even theoretically reachable in practice, but there's not much point in erroneous error checks. Discussion: https://postgr.es/m/[email protected] Branch -- REL9_3_STABLE Details --- https://git.postgresql.org/pg/commitdiff/3531365de5e868224c70cd568384f453056f7646 Modified Files -- src/backend/libpq/ip.c | 6 +++--- src/backend/postmaster/pgstat.c | 2 +- src/interfaces/ecpg/pgtypeslib/common.c | 2 +- src/port/getaddrinfo.c | 2 +- src/test/isolation/isolation_main.c | 22 +- src/test/regress/pg_regress.c | 2 +- src/test/regress/pg_regress_main.c | 26 +++--- 7 files changed, 43 insertions(+), 19 deletions(-)
pgsql: Clean up assorted misuses of snprintf()'s result value.
Clean up assorted misuses of snprintf()'s result value. Fix a small number of places that were testing the result of snprintf() but doing so incorrectly. The right test for buffer overrun, per C99, is "result >= bufsize" not "result > bufsize". Some places were also checking for failure with "result == -1", but the standard only says that a negative value is delivered on failure. (Note that this only makes these places correct if snprintf() delivers C99-compliant results. But at least now these places are consistent with all the other places where we assume that.) Also, make psql_start_test() and isolation_start_test() check for buffer overrun while constructing their shell commands. There seems like a higher risk of overrun, with more severe consequences, here than there is for the individual file paths that are made elsewhere in the same functions, so this seemed like a worthwhile change. Also fix guc.c's do_serialize() to initialize errno = 0 before calling vsnprintf. In principle, this should be unnecessary because vsnprintf should have set errno if it returns a failure indication ... but the other two places this coding pattern is cribbed from don't assume that, so let's be consistent. These errors are all very old, so back-patch as appropriate. I think that only the shell command overrun cases are even theoretically reachable in practice, but there's not much point in erroneous error checks. Discussion: https://postgr.es/m/[email protected] Branch -- REL9_4_STABLE Details --- https://git.postgresql.org/pg/commitdiff/d371efb39c33f79ad5f6741d76bfae54df21eb55 Modified Files -- src/backend/libpq/ip.c | 6 +++--- src/backend/postmaster/pgstat.c | 2 +- src/interfaces/ecpg/pgtypeslib/common.c | 2 +- src/port/getaddrinfo.c | 2 +- src/test/isolation/isolation_main.c | 24 ++-- src/test/regress/pg_regress.c | 2 +- src/test/regress/pg_regress_main.c | 26 +++--- 7 files changed, 44 insertions(+), 20 deletions(-)
pgsql: Clean up assorted misuses of snprintf()'s result value.
Clean up assorted misuses of snprintf()'s result value. Fix a small number of places that were testing the result of snprintf() but doing so incorrectly. The right test for buffer overrun, per C99, is "result >= bufsize" not "result > bufsize". Some places were also checking for failure with "result == -1", but the standard only says that a negative value is delivered on failure. (Note that this only makes these places correct if snprintf() delivers C99-compliant results. But at least now these places are consistent with all the other places where we assume that.) Also, make psql_start_test() and isolation_start_test() check for buffer overrun while constructing their shell commands. There seems like a higher risk of overrun, with more severe consequences, here than there is for the individual file paths that are made elsewhere in the same functions, so this seemed like a worthwhile change. Also fix guc.c's do_serialize() to initialize errno = 0 before calling vsnprintf. In principle, this should be unnecessary because vsnprintf should have set errno if it returns a failure indication ... but the other two places this coding pattern is cribbed from don't assume that, so let's be consistent. These errors are all very old, so back-patch as appropriate. I think that only the shell command overrun cases are even theoretically reachable in practice, but there's not much point in erroneous error checks. Discussion: https://postgr.es/m/[email protected] Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/d7ed4eea539dbd6bf89163213ea6068b4ef0fe11 Modified Files -- src/backend/postmaster/pgstat.c | 2 +- src/backend/utils/misc/guc.c| 2 ++ src/common/ip.c | 6 +++--- src/interfaces/ecpg/pgtypeslib/common.c | 2 +- src/port/getaddrinfo.c | 2 +- src/test/isolation/isolation_main.c | 24 ++-- src/test/regress/pg_regress.c | 2 +- src/test/regress/pg_regress_main.c | 28 8 files changed, 47 insertions(+), 21 deletions(-)
pgsql: Clean up assorted misuses of snprintf()'s result value.
Clean up assorted misuses of snprintf()'s result value. Fix a small number of places that were testing the result of snprintf() but doing so incorrectly. The right test for buffer overrun, per C99, is "result >= bufsize" not "result > bufsize". Some places were also checking for failure with "result == -1", but the standard only says that a negative value is delivered on failure. (Note that this only makes these places correct if snprintf() delivers C99-compliant results. But at least now these places are consistent with all the other places where we assume that.) Also, make psql_start_test() and isolation_start_test() check for buffer overrun while constructing their shell commands. There seems like a higher risk of overrun, with more severe consequences, here than there is for the individual file paths that are made elsewhere in the same functions, so this seemed like a worthwhile change. Also fix guc.c's do_serialize() to initialize errno = 0 before calling vsnprintf. In principle, this should be unnecessary because vsnprintf should have set errno if it returns a failure indication ... but the other two places this coding pattern is cribbed from don't assume that, so let's be consistent. These errors are all very old, so back-patch as appropriate. I think that only the shell command overrun cases are even theoretically reachable in practice, but there's not much point in erroneous error checks. Discussion: https://postgr.es/m/[email protected] Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/cc4f6b77861803be99dfc17a38052035a0af5ae6 Modified Files -- src/backend/postmaster/pgstat.c | 2 +- src/backend/utils/misc/guc.c| 2 ++ src/common/ip.c | 6 +++--- src/interfaces/ecpg/pgtypeslib/common.c | 2 +- src/port/getaddrinfo.c | 2 +- src/test/isolation/isolation_main.c | 24 ++-- src/test/regress/pg_regress.c | 2 +- src/test/regress/pg_regress_main.c | 28 8 files changed, 47 insertions(+), 21 deletions(-)
pgsql: Clean up assorted misuses of snprintf()'s result value.
Clean up assorted misuses of snprintf()'s result value. Fix a small number of places that were testing the result of snprintf() but doing so incorrectly. The right test for buffer overrun, per C99, is "result >= bufsize" not "result > bufsize". Some places were also checking for failure with "result == -1", but the standard only says that a negative value is delivered on failure. (Note that this only makes these places correct if snprintf() delivers C99-compliant results. But at least now these places are consistent with all the other places where we assume that.) Also, make psql_start_test() and isolation_start_test() check for buffer overrun while constructing their shell commands. There seems like a higher risk of overrun, with more severe consequences, here than there is for the individual file paths that are made elsewhere in the same functions, so this seemed like a worthwhile change. Also fix guc.c's do_serialize() to initialize errno = 0 before calling vsnprintf. In principle, this should be unnecessary because vsnprintf should have set errno if it returns a failure indication ... but the other two places this coding pattern is cribbed from don't assume that, so let's be consistent. These errors are all very old, so back-patch as appropriate. I think that only the shell command overrun cases are even theoretically reachable in practice, but there's not much point in erroneous error checks. Discussion: https://postgr.es/m/[email protected] Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/c182c1e0b895c8e6baf7c5a9d3cd98307f420168 Modified Files -- src/backend/libpq/ip.c | 6 +++--- src/backend/postmaster/pgstat.c | 2 +- src/backend/utils/misc/guc.c| 2 ++ src/interfaces/ecpg/pgtypeslib/common.c | 2 +- src/port/getaddrinfo.c | 2 +- src/test/isolation/isolation_main.c | 24 ++-- src/test/regress/pg_regress.c | 2 +- src/test/regress/pg_regress_main.c | 26 +++--- 8 files changed, 46 insertions(+), 20 deletions(-)
pgsql: Update FSM on WAL replay of page all-visible/frozen
Update FSM on WAL replay of page all-visible/frozen We aren't very strict about keeping FSM up to date on WAL replay, because per-page freespace values aren't critical in replicas (can't write to heap in a replica; and if the replica is promoted, the values would be updated by VACUUM anyway). However, VACUUM since 9.6 can skip processing pages marked all-visible or all-frozen, and if such pages are recorded in FSM with wrong values, those values are blindly propagated to FSM's upper layers by VACUUM's FreeSpaceMapVacuum. (This rationale assumes that crashes are not very frequent, because those would cause outdated FSM to occur in the primary.) Even when the FSM is outdated in standby, things are not too bad normally, because, most per-page FSM values will be zero (other than those propagated with the base-backup that created the standby); only once the remaining free space is less than 0.2*BLCKSZ the per-page value is maintained by WAL replay of heap ins/upd/del. However, if wal_log_hints=on causes complete FSM pages to be propagated to a standby via full-page images, many too-optimistic per-page values can end up being registered in the standby. Incorrect per-page values aren't critical in most cases, since an inserter that is given a page that doesn't actually contain the claimed free space will update FSM with the correct value, and retry until it finds a usable page. However, if there are many such updates to do, an inserter can spend a long time doing them before a usable page is found; in a heavily trafficked insert-only table with many concurrent inserters this has been observed to cause several second stalls, causing visible application malfunction. To fix this problem, it seems sufficient to have heap_xlog_visible (replay of setting all-visible and all-frozen VM bits for a heap page) update the FSM value for the page being processed. This fixes the per-page counters together with making the page skippable to vacuum, so when vacuum does FreeSpaceMapVacuum, the values propagated to FSM upper layers are the correct ones, avoiding the problem. While at it, apply the same fix to heap_xlog_clean (replay of tuple removal by HOT pruning and vacuum). This makes any space freed by the cleaning available earlier than the next vacuum in the promoted replica. Backpatch to 9.6, where this problem was diagnosed on an insert-only table with all-frozen pages, which were introduced as a concept in that release. Theoretically it could apply with all-visible pages to older branches, but there's been no report of that and it doesn't backpatch cleanly anyway. Author: Álvaro Herrera Discussion: https://postgr.es/m/[email protected] Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/ab7dbd681c54d993fc8ebf8a413668fd75a4be0b Modified Files -- src/backend/access/heap/heapam.c | 52 ++-- 1 file changed, 40 insertions(+), 12 deletions(-)
pgsql: Make snprintf.c follow the C99 standard for snprintf's result va
Make snprintf.c follow the C99 standard for snprintf's result value. C99 says that the result should be the number of bytes that would have been emitted given a large enough buffer, not the number we actually were able to put in the buffer. It's time to make our substitute implementation comply with that. Not doing so results in inefficiency in buffer-enlargement cases, and also poses a portability hazard for third-party code that might expect C99-compliant snprintf behavior within Postgres. In passing, remove useless tests for str == NULL; neither C99 nor predecessor standards ever allowed that except when count == 0, so I see no reason to expend cycles on making that a non-crash case for this implementation. Also, don't waste a byte in pg_vfprintf's local I/O buffer; this might have performance benefits by allowing aligned writes during flushbuffer calls. Back-patch of commit 805889d7d. There was some concern about this possibly breaking code that assumes pre-C99 behavior, but there is much more risk (and reality, in our own code) of code that assumes C99 behavior and hence fails to detect buffer overrun without this. Discussion: https://postgr.es/m/[email protected] Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/c2a2e331da17e86ba874ff9449b1d2c9e7e829fd Modified Files -- src/port/snprintf.c | 94 +++-- 1 file changed, 63 insertions(+), 31 deletions(-)
pgsql: Update FSM on WAL replay of page all-visible/frozen
Update FSM on WAL replay of page all-visible/frozen We aren't very strict about keeping FSM up to date on WAL replay, because per-page freespace values aren't critical in replicas (can't write to heap in a replica; and if the replica is promoted, the values would be updated by VACUUM anyway). However, VACUUM since 9.6 can skip processing pages marked all-visible or all-frozen, and if such pages are recorded in FSM with wrong values, those values are blindly propagated to FSM's upper layers by VACUUM's FreeSpaceMapVacuum. (This rationale assumes that crashes are not very frequent, because those would cause outdated FSM to occur in the primary.) Even when the FSM is outdated in standby, things are not too bad normally, because, most per-page FSM values will be zero (other than those propagated with the base-backup that created the standby); only once the remaining free space is less than 0.2*BLCKSZ the per-page value is maintained by WAL replay of heap ins/upd/del. However, if wal_log_hints=on causes complete FSM pages to be propagated to a standby via full-page images, many too-optimistic per-page values can end up being registered in the standby. Incorrect per-page values aren't critical in most cases, since an inserter that is given a page that doesn't actually contain the claimed free space will update FSM with the correct value, and retry until it finds a usable page. However, if there are many such updates to do, an inserter can spend a long time doing them before a usable page is found; in a heavily trafficked insert-only table with many concurrent inserters this has been observed to cause several second stalls, causing visible application malfunction. To fix this problem, it seems sufficient to have heap_xlog_visible (replay of setting all-visible and all-frozen VM bits for a heap page) update the FSM value for the page being processed. This fixes the per-page counters together with making the page skippable to vacuum, so when vacuum does FreeSpaceMapVacuum, the values propagated to FSM upper layers are the correct ones, avoiding the problem. While at it, apply the same fix to heap_xlog_clean (replay of tuple removal by HOT pruning and vacuum). This makes any space freed by the cleaning available earlier than the next vacuum in the promoted replica. Backpatch to 9.6, where this problem was diagnosed on an insert-only table with all-frozen pages, which were introduced as a concept in that release. Theoretically it could apply with all-visible pages to older branches, but there's been no report of that and it doesn't backpatch cleanly anyway. Author: Álvaro Herrera Discussion: https://postgr.es/m/[email protected] Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/3cbd190e1579546bd89b650d9d388ae3e80b4eae Modified Files -- src/backend/access/heap/heapam.c | 52 ++-- 1 file changed, 40 insertions(+), 12 deletions(-)
pgsql: Make snprintf.c follow the C99 standard for snprintf's result va
Make snprintf.c follow the C99 standard for snprintf's result value. C99 says that the result should be the number of bytes that would have been emitted given a large enough buffer, not the number we actually were able to put in the buffer. It's time to make our substitute implementation comply with that. Not doing so results in inefficiency in buffer-enlargement cases, and also poses a portability hazard for third-party code that might expect C99-compliant snprintf behavior within Postgres. In passing, remove useless tests for str == NULL; neither C99 nor predecessor standards ever allowed that except when count == 0, so I see no reason to expend cycles on making that a non-crash case for this implementation. Also, don't waste a byte in pg_vfprintf's local I/O buffer; this might have performance benefits by allowing aligned writes during flushbuffer calls. Back-patch of commit 805889d7d. There was some concern about this possibly breaking code that assumes pre-C99 behavior, but there is much more risk (and reality, in our own code) of code that assumes C99 behavior and hence fails to detect buffer overrun without this. Discussion: https://postgr.es/m/[email protected] Branch -- REL9_4_STABLE Details --- https://git.postgresql.org/pg/commitdiff/27c4b0899c0e44259b0ab27ced56490c669e329c Modified Files -- src/port/snprintf.c | 94 +++-- 1 file changed, 63 insertions(+), 31 deletions(-)
pgsql: Make snprintf.c follow the C99 standard for snprintf's result va
Make snprintf.c follow the C99 standard for snprintf's result value. C99 says that the result should be the number of bytes that would have been emitted given a large enough buffer, not the number we actually were able to put in the buffer. It's time to make our substitute implementation comply with that. Not doing so results in inefficiency in buffer-enlargement cases, and also poses a portability hazard for third-party code that might expect C99-compliant snprintf behavior within Postgres. In passing, remove useless tests for str == NULL; neither C99 nor predecessor standards ever allowed that except when count == 0, so I see no reason to expend cycles on making that a non-crash case for this implementation. Also, don't waste a byte in pg_vfprintf's local I/O buffer; this might have performance benefits by allowing aligned writes during flushbuffer calls. Back-patch of commit 805889d7d. There was some concern about this possibly breaking code that assumes pre-C99 behavior, but there is much more risk (and reality, in our own code) of code that assumes C99 behavior and hence fails to detect buffer overrun without this. Discussion: https://postgr.es/m/[email protected] Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/36147ec9f1e2dcbe35fc813825242d72d1c57b70 Modified Files -- src/port/snprintf.c | 94 +++-- 1 file changed, 63 insertions(+), 31 deletions(-)
pgsql: Make snprintf.c follow the C99 standard for snprintf's result va
Make snprintf.c follow the C99 standard for snprintf's result value. C99 says that the result should be the number of bytes that would have been emitted given a large enough buffer, not the number we actually were able to put in the buffer. It's time to make our substitute implementation comply with that. Not doing so results in inefficiency in buffer-enlargement cases, and also poses a portability hazard for third-party code that might expect C99-compliant snprintf behavior within Postgres. In passing, remove useless tests for str == NULL; neither C99 nor predecessor standards ever allowed that except when count == 0, so I see no reason to expend cycles on making that a non-crash case for this implementation. Also, don't waste a byte in pg_vfprintf's local I/O buffer; this might have performance benefits by allowing aligned writes during flushbuffer calls. Back-patch of commit 805889d7d. There was some concern about this possibly breaking code that assumes pre-C99 behavior, but there is much more risk (and reality, in our own code) of code that assumes C99 behavior and hence fails to detect buffer overrun without this. Discussion: https://postgr.es/m/[email protected] Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/1811900b933c892a8ee102b8b62028de4c1379ef Modified Files -- src/port/snprintf.c | 94 +++-- 1 file changed, 63 insertions(+), 31 deletions(-)
pgsql: Make snprintf.c follow the C99 standard for snprintf's result va
Make snprintf.c follow the C99 standard for snprintf's result value. C99 says that the result should be the number of bytes that would have been emitted given a large enough buffer, not the number we actually were able to put in the buffer. It's time to make our substitute implementation comply with that. Not doing so results in inefficiency in buffer-enlargement cases, and also poses a portability hazard for third-party code that might expect C99-compliant snprintf behavior within Postgres. In passing, remove useless tests for str == NULL; neither C99 nor predecessor standards ever allowed that except when count == 0, so I see no reason to expend cycles on making that a non-crash case for this implementation. Also, don't waste a byte in pg_vfprintf's local I/O buffer; this might have performance benefits by allowing aligned writes during flushbuffer calls. Back-patch of commit 805889d7d. There was some concern about this possibly breaking code that assumes pre-C99 behavior, but there is much more risk (and reality, in our own code) of code that assumes C99 behavior and hence fails to detect buffer overrun without this. Discussion: https://postgr.es/m/[email protected] Branch -- REL9_5_STABLE Details --- https://git.postgresql.org/pg/commitdiff/8e9f229d2bf668e780ce10d511925b11855afd2c Modified Files -- src/port/snprintf.c | 94 +++-- 1 file changed, 63 insertions(+), 31 deletions(-)
pgsql: Update FSM on WAL replay of page all-visible/frozen
Update FSM on WAL replay of page all-visible/frozen We aren't very strict about keeping FSM up to date on WAL replay, because per-page freespace values aren't critical in replicas (can't write to heap in a replica; and if the replica is promoted, the values would be updated by VACUUM anyway). However, VACUUM since 9.6 can skip processing pages marked all-visible or all-frozen, and if such pages are recorded in FSM with wrong values, those values are blindly propagated to FSM's upper layers by VACUUM's FreeSpaceMapVacuum. (This rationale assumes that crashes are not very frequent, because those would cause outdated FSM to occur in the primary.) Even when the FSM is outdated in standby, things are not too bad normally, because, most per-page FSM values will be zero (other than those propagated with the base-backup that created the standby); only once the remaining free space is less than 0.2*BLCKSZ the per-page value is maintained by WAL replay of heap ins/upd/del. However, if wal_log_hints=on causes complete FSM pages to be propagated to a standby via full-page images, many too-optimistic per-page values can end up being registered in the standby. Incorrect per-page values aren't critical in most cases, since an inserter that is given a page that doesn't actually contain the claimed free space will update FSM with the correct value, and retry until it finds a usable page. However, if there are many such updates to do, an inserter can spend a long time doing them before a usable page is found; in a heavily trafficked insert-only table with many concurrent inserters this has been observed to cause several second stalls, causing visible application malfunction. To fix this problem, it seems sufficient to have heap_xlog_visible (replay of setting all-visible and all-frozen VM bits for a heap page) update the FSM value for the page being processed. This fixes the per-page counters together with making the page skippable to vacuum, so when vacuum does FreeSpaceMapVacuum, the values propagated to FSM upper layers are the correct ones, avoiding the problem. While at it, apply the same fix to heap_xlog_clean (replay of tuple removal by HOT pruning and vacuum). This makes any space freed by the cleaning available earlier than the next vacuum in the promoted replica. Backpatch to 9.6, where this problem was diagnosed on an insert-only table with all-frozen pages, which were introduced as a concept in that release. Theoretically it could apply with all-visible pages to older branches, but there's been no report of that and it doesn't backpatch cleanly anyway. Author: Álvaro Herrera Discussion: https://postgr.es/m/[email protected] Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/255e2fbe8fa68277c7e2f5339ea2ca05b899ffc4 Modified Files -- src/backend/access/heap/heapam.c | 52 ++-- 1 file changed, 40 insertions(+), 12 deletions(-)
pgsql: Update FSM on WAL replay of page all-visible/frozen
Update FSM on WAL replay of page all-visible/frozen We aren't very strict about keeping FSM up to date on WAL replay, because per-page freespace values aren't critical in replicas (can't write to heap in a replica; and if the replica is promoted, the values would be updated by VACUUM anyway). However, VACUUM since 9.6 can skip processing pages marked all-visible or all-frozen, and if such pages are recorded in FSM with wrong values, those values are blindly propagated to FSM's upper layers by VACUUM's FreeSpaceMapVacuum. (This rationale assumes that crashes are not very frequent, because those would cause outdated FSM to occur in the primary.) Even when the FSM is outdated in standby, things are not too bad normally, because, most per-page FSM values will be zero (other than those propagated with the base-backup that created the standby); only once the remaining free space is less than 0.2*BLCKSZ the per-page value is maintained by WAL replay of heap ins/upd/del. However, if wal_log_hints=on causes complete FSM pages to be propagated to a standby via full-page images, many too-optimistic per-page values can end up being registered in the standby. Incorrect per-page values aren't critical in most cases, since an inserter that is given a page that doesn't actually contain the claimed free space will update FSM with the correct value, and retry until it finds a usable page. However, if there are many such updates to do, an inserter can spend a long time doing them before a usable page is found; in a heavily trafficked insert-only table with many concurrent inserters this has been observed to cause several second stalls, causing visible application malfunction. To fix this problem, it seems sufficient to have heap_xlog_visible (replay of setting all-visible and all-frozen VM bits for a heap page) update the FSM value for the page being processed. This fixes the per-page counters together with making the page skippable to vacuum, so when vacuum does FreeSpaceMapVacuum, the values propagated to FSM upper layers are the correct ones, avoiding the problem. While at it, apply the same fix to heap_xlog_clean (replay of tuple removal by HOT pruning and vacuum). This makes any space freed by the cleaning available earlier than the next vacuum in the promoted replica. Backpatch to 9.6, where this problem was diagnosed on an insert-only table with all-frozen pages, which were introduced as a concept in that release. Theoretically it could apply with all-visible pages to older branches, but there's been no report of that and it doesn't backpatch cleanly anyway. Author: Álvaro Herrera Discussion: https://postgr.es/m/[email protected] Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/6872c2be6a97057aa736110e31f0390a53305c9e Modified Files -- src/backend/access/heap/heapam.c | 52 ++-- 1 file changed, 40 insertions(+), 12 deletions(-)
pgsql: Make snprintf.c follow the C99 standard for snprintf's result va
Make snprintf.c follow the C99 standard for snprintf's result value. C99 says that the result should be the number of bytes that would have been emitted given a large enough buffer, not the number we actually were able to put in the buffer. It's time to make our substitute implementation comply with that. Not doing so results in inefficiency in buffer-enlargement cases, and also poses a portability hazard for third-party code that might expect C99-compliant snprintf behavior within Postgres. In passing, remove useless tests for str == NULL; neither C99 nor predecessor standards ever allowed that except when count == 0, so I see no reason to expend cycles on making that a non-crash case for this implementation. Also, don't waste a byte in pg_vfprintf's local I/O buffer; this might have performance benefits by allowing aligned writes during flushbuffer calls. Back-patch of commit 805889d7d. There was some concern about this possibly breaking code that assumes pre-C99 behavior, but there is much more risk (and reality, in our own code) of code that assumes C99 behavior and hence fails to detect buffer overrun without this. Discussion: https://postgr.es/m/[email protected] Branch -- REL9_3_STABLE Details --- https://git.postgresql.org/pg/commitdiff/a57a6faf601186ef1b3c4f1149e5520ea98cbb76 Modified Files -- src/port/snprintf.c | 94 +++-- 1 file changed, 63 insertions(+), 31 deletions(-)
pgsql: Improve comment in GetNewObjectId().
Improve comment in GetNewObjectId(). The previous comment gave the impression that skipping OIDs before FirstNormalObjectId was merely an optimization to avoid likely collisions. In fact other parts of the system have been relying on this threshold to detect system-created objects since commit 8e18d04d4da, so adjust the wording. Author: Thomas Munro Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAEepm%3D33JASACeOayr_W3%3DCSjy2jiPxM-k89axu0akFbHdjnjA%40mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/ca1e64febaf57eb5f3f24340c5bcce5430cad7a5 Modified Files -- src/backend/access/transam/varsup.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-)
