From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> |
Cc: | Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>, Prabhat Sahu <prabhat(dot)sahu(at)enterprisedb(dot)com>, Peter Geoghegan <pg(at)bowt(dot)ie>, Robert Haas <robertmhaas(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Rafia Sabih <rafia(dot)sabih(at)enterprisedb(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Oleg Golovanov <rentech(at)mail(dot)ru> |
Subject: | Re: [HACKERS] Parallel Hash take II |
Date: | 2017-12-18 22:44:22 |
Message-ID: | 20171218224422.smslql7js4meqw2t@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2017-12-14 21:06:34 +1300, Thomas Munro wrote:
> On Thu, Dec 14, 2017 at 11:45 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> > + if (accessor->write_pointer + accessor->sts->meta_data_size >=
> > + accessor->write_end)
> > + elog(ERROR, "meta-data too long");
> >
> > That seems more like an Assert than a proper elog? Given that we're
> > calculating size just a few lines above...
>
> It's an error because the logic is not smart enough to split the
> optional meta-data and tuple size over multiple chunks. I have added
> comments there to explain.
I don't see how that requires it to be an elog rather than an assert. As
far as I can tell this is only reachable if
meta_data_size + sizeof(uint32) >= STS_CHUNK_DATA_SIZE. For anything
smaller the sts_flush_chunk() a few lines above would have started a new
chunk.
IOW, we should detect this at sts_initialize() time, elog there, and
Assert() out in puttuple.
I've changed the code like that, fixed my own < vs >= confusion during
the conversion, fixed a typo, and pushed. If you're not ok with that
change, we can easily whack this around.
I've not changed this, but I wonder whether we should rename
sts_puttuple() to sts_put_tuple(), for consistency with
sts_read_tuple(). Or the other way round. Or...
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | Masahiko Sawada | 2017-12-19 00:22:28 | Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager |
Previous Message | Bossart, Nathan | 2017-12-18 22:18:09 | Re: BUG #14941: Vacuum crashes |