On Thu, Jun 21, 2018 at 4:26 PM, Vik Fearing <vik.fear...@2ndquadrant.com> wrote:
> On 21/06/18 07:27, Michael Paquier wrote: > > Attached is a patch which includes your suggestion. What do you think? > > As that's an improvement, only HEAD would get that clarification. > > Say what? If the clarification applies to previous versions, as it > does, it should be backpatched. This isn't a change in behavior, it's a > change in the description of existing behavior. > Generally only actual bug fixes get back-patched; but I'd have to say this looks like it could easily be classified as one. Before: These are backups that cannot be used for PITR After: These are backups that could be used for PITR if ... Changing a cannot to a can seems like we are fixing a bug in the documentation. Some comments on the patch itself: "recover up to the wanted recovery point." - "desired recovery point" reads better to me ==== "These backups are typically much faster to backup and restore" - "These backups are typically much faster to create and restore"; avoid repeated use of the word backup "but can result as well in larger backup sizes" - "but can result in larger backup sizes", drop the unnecessary 'as well' "sizes, so the speed of one method or the other is to evaluate carefully first" - that is just wrong as-is; suggest just removing it. ==== To cover the last three items as a whole I'd suggest: "These backups are typically much faster to create and restore, but generate larger file sizes, compared to pg_dump." For the last sentence I'd suggest: "Note that because WAL cannot be applied on top of a restored pg_dump backup it is considered a cold backup and cannot be used for point-in-time-recovery." I like adding "cold backup" here to help contrast and explain why a base backup is considered a "hot backup". The rest is style to make that flow better. David J.