Re: backup manifests

On 3/31/20 7:57 AM, Robert Haas wrote:
> On Mon, Mar 30, 2020 at 7:24 PM David Steele <david@pgmasters.net> wrote:
>>> I'm confused as to why you're not seeing that. What's the exact
>>> sequence of steps?
>>
>> $ pg_basebackup -D test/backup5 --manifest-checksums=SHA256
>>
>> $ vi test/backup5/backup_manifest
>>       * Add 'X' to the checksum of backup_label
>>
>> $ pg_validatebackup test/backup5
>> pg_validatebackup: fatal: invalid checksum for file "backup_label":
>> "a98e9164fd59d498d14cfdf19c67d1c2208a30e7b939d1b4a09f524c7adfc11fX"
>>
>> No mention of the manifest checksum being invalid.  But if I remove the
>> backup label file from the manifest:
>>
>> pg_validatebackup: fatal: manifest checksum mismatch
> 
> Oh, I see what's happening now. If the checksum is not an even-length
> string of hexademical characters, it's treated as a syntax error, so
> it bails out at that point. Generally, a syntax error in the manifest
> file is treated as a fatal error, and you just die right there. You'd
> get the same behavior if you had malformed JSON, like a stray { or }
> or [ or ] someplace that it doesn't belong according to the rules of
> JSON. On the other hand, if you corrupt the checksum by adding AA or
> EE or 54 or some other even-length string of hex characters, then you
> have (in this code's view) a semantic error rather than a syntax
> error, so it will finish loading all the manifest data and then bail
> because the checksum doesn't match.
> 
> We really can't avoid bailing out early sometimes, because if the file
> is totally malformed at the JSON level, there's just no way to
> continue. We could cause this particular error to get treated as a
> semantic error rather than a syntax error, but I don't really see much
> advantage in so doing. This way was easier to code, and I don't think
> it really matters which error we find first.

I think it would be good to know that the manifest checksum is bad in 
all cases because that may well inform other errors.

That said, I know you have a lot on your plate with this patch so I'm 
not going to make a fuss about such a minor gripe. Perhaps this can be 
considered for future improvement.

Regards,
-- 
-David
david@pgmasters.net