On Wed, Mar 23, 2016 at 3:43 PM, Pavan Deolasee <pavan.deola...@gmail.com> wrote: > While investigating some issue, I found that pg_xlogdump fails to dump > contents from a WAL file if the file has continuation data from previous WAL > record and the data spans more than one page. In such cases, > XLogFindNextRecord() fails to take into account that there will be more than > one xlog page headers (long and short) and thus tries to read from an offset > where no valid record exists. That results in pg_xlogdump throwing error > such as:
OK. That makes sense, that is indeed a possible scenario. > Attached WAL file from master branch demonstrates the issue, generated using > synthetic data. Also, attached patch fixes it for me. So does it for me. > While we could have deduced the number of short and long headers and skipped > directly to the offset, I found reading one page at a time and using > XLogPageHeaderSize() to find header size of each page separately, a much > cleaner way. Also, the continuation data is not going to span many pages. So > I don't see any harm in doing it that way. I have to agree, the new code is pretty clean this way by calculating the next page LSN directly in the loop should the record be longer than it. > I encountered this on 9.3, but the patch applies to both 9.3 and master. I > haven't tested it on other branches, but I have no reason to believe it > won't apply or work. I believe we should back patch it all supported > branches. Agreed, that's a bug, and the logic of your patch looks good to me. + /* + * Compute targetRecOff. It should typically be greater than short + * page-header since a valid record can't , but can also be zero when + * caller has supplied a page-aligned address or when we are skipping + * multi-page continuation record. It doesn't matter though because + * ReadPageInternal() will read at least short page-header worth of + * data + */ This needs some polishing, there is an unfinished sentence here. + targetRecOff = tmpRecPtr % XLOG_BLCKSZ; targetRecOff, pageHeaderSize and targetPagePtr could be declared inside directly the new while loop. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
