Re: [HACKERS] Subtle bug in "Simplify tape block format" commit

2017-02-01 Thread Heikki Linnakangas
On 01/31/2017 01:51 AM, Peter Geoghegan wrote: * If you're writing out any old bit pattern, I suggest using 0x7F bytes rather than nul bytes (I do agree that it does seem worth making this some particular bit pattern, so as to not have to bother with Valgrind suppressions and so on). That

Re: [HACKERS] Subtle bug in "Simplify tape block format" commit

2017-01-30 Thread Peter Geoghegan
On Mon, Jan 30, 2017 at 10:01 AM, Peter Geoghegan wrote: > Let me take another look at this later today before proceeding. I want > to run it against a custom test suite I've developed. I've done so. Some more thoughts: * I don't think that this is really any less efficient than

Re: [HACKERS] Subtle bug in "Simplify tape block format" commit

2017-01-30 Thread Peter Geoghegan
On Mon, Jan 30, 2017 at 9:55 AM, Heikki Linnakangas wrote: > It won't make any difference for correctness what bit pattern you write to > "fill the hole", but you have to write something, and zeros seems like a > natural choice. Ah, okay. That's probably fine, then. Let me take

Re: [HACKERS] Subtle bug in "Simplify tape block format" commit

2017-01-30 Thread Heikki Linnakangas
On 01/30/2017 07:45 PM, Peter Geoghegan wrote: On Mon, Jan 30, 2017 at 4:04 AM, Heikki Linnakangas wrote: Alternatively, we could fix this with a small change in ltsWriteBlock(), see attached patch. When we're about to create a hole in the file, write all-zero blocks to avoid

Re: [HACKERS] Subtle bug in "Simplify tape block format" commit

2017-01-30 Thread Peter Geoghegan
On Mon, Jan 30, 2017 at 4:04 AM, Heikki Linnakangas wrote: > Hmm. The LogicalTapeEndWriting() function is very similar to the > LogicalTapePause() function I had in the pause/resume patch. They both flush > the last block to disk. The difference is that LogicalTapePause() also >

Re: [HACKERS] Subtle bug in "Simplify tape block format" commit

2017-01-30 Thread Heikki Linnakangas
On 01/07/2017 12:33 AM, Peter Geoghegan wrote: Previously, all calls to ltsGetFreeBlock() were immediately followed by a corresponding call to ltsWriteBlock(); we wrote out the newly-allocated-in-first-pass block there and then. It's a good idea for a corresponding ltsWriteBlock() call to happen

[HACKERS] Subtle bug in "Simplify tape block format" commit

2017-01-06 Thread Peter Geoghegan
It seems that commit 01ec2563 has a subtle bug, which stems from the fact that logtape.c no longer follows the rule described above ltsGetFreeBlock(): /* * Select a currently unused block for writing to. * * NB: should only be called when writer is ready to write immediately, * to ensure that