Обсуждение: Why we panic in pglz_decompress
I'm now looking into toast code and I found following code in
pglz_decompress:
00704 if (destsize != source->rawsize)
00705 elog(destsize > source->rawsize ? FATAL : ERROR,
00706 "compressed data is corrupt");
I'm surprise why we there panic? By my opinion is not too good idea to
crash server in case when we know how much memory we really have for
dest and we can check range. Other silly thing is that message
"compressed data is corrupt" does not contain any information about file
relation etc.
My idea is to improve this piece of code and move error logging to
callers (heap_tuple_untoast_attr() and heap_tuple_untoast_attr_slice())
where we have a little bit more details (especially for external storage).
Any comments?
thanks Zdenek
Zdenek Kotala wrote: > I'm now looking into toast code and I found following code in > pglz_decompress: > > 00704 if (destsize != source->rawsize) > 00705 elog(destsize > source->rawsize ? FATAL : ERROR, > 00706 "compressed data is corrupt"); > > > I'm surprise why we there panic? Agreed, FATAL is too strong. > My idea is to improve this piece of code and move error logging to > callers (heap_tuple_untoast_attr() and heap_tuple_untoast_attr_slice()) > where we have a little bit more details (especially for external > storage). Why move it? Just adding errcontext in the callers should be enough. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera napsal(a):
> Zdenek Kotala wrote:
>> I'm now looking into toast code and I found following code in
>> pglz_decompress:
>>
>> 00704 if (destsize != source->rawsize)
>> 00705 elog(destsize > source->rawsize ? FATAL : ERROR,
>> 00706 "compressed data is corrupt");
>>
>>
>> I'm surprise why we there panic?
>
> Agreed, FATAL is too strong.
>
>> My idea is to improve this piece of code and move error logging to
>> callers (heap_tuple_untoast_attr() and heap_tuple_untoast_attr_slice())
>> where we have a little bit more details (especially for external
>> storage).
>
> Why move it? Just adding errcontext in the callers should be enough.
Good idea.
thanks Zdenek
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Zdenek Kotala wrote:
>> I'm now looking into toast code and I found following code in
>> pglz_decompress:
>>
>> 00704 if (destsize != source->rawsize)
>> 00705 elog(destsize > source->rawsize ? FATAL : ERROR,
>> 00706 "compressed data is corrupt");
>>
>>
>> I'm surprise why we there panic?
> Agreed, FATAL is too strong.
Did either of you read the comment just before this code? The reason
it's panicing is that it's possibly already tromped on some critical
data structure inside the backend.
>> My idea is to improve this piece of code and move error logging to
>> callers (heap_tuple_untoast_attr() and heap_tuple_untoast_attr_slice())
>> where we have a little bit more details (especially for external
>> storage).
> Why move it? Just adding errcontext in the callers should be enough.
AFAIR this error has never once been reported from the field, so I don't
see the point of investing a lot of effort in it.
regards, tom lane
Tom Lane napsal(a):
> Alvaro Herrera <alvherre@commandprompt.com> writes:
>> Zdenek Kotala wrote:
>>> I'm now looking into toast code and I found following code in
>>> pglz_decompress:
>>>
>>> 00704 if (destsize != source->rawsize)
>>> 00705 elog(destsize > source->rawsize ? FATAL : ERROR,
>>> 00706 "compressed data is corrupt");
>>>
>>>
>>> I'm surprise why we there panic?
>
>> Agreed, FATAL is too strong.
>
> Did either of you read the comment just before this code? The reason
> it's panicing is that it's possibly already tromped on some critical
> data structure inside the backend.
Yes I did, but if you know how big memory you have for uncompress data
you can check a boundaries. It is better then overwrite a data in
memory. Yes, it little bit slow down a routine but you will able work
with a table.
>>> My idea is to improve this piece of code and move error logging to
>>> callers (heap_tuple_untoast_attr() and heap_tuple_untoast_attr_slice())
>>> where we have a little bit more details (especially for external
>>> storage).
>
>> Why move it? Just adding errcontext in the callers should be enough.
>
> AFAIR this error has never once been reported from the field, so I don't
> see the point of investing a lot of effort in it.
Please, increment a counter :-). I'm now analyzing one core file and it
fails finally in elog function (called from pglz_decompress), because
memory was overwritten -> no error message in a log file. :(
Zdenek
Need help and direction creating new aggregate functions. We need to add more average functions for both scientific and finical purposes RMS for electrical measurement purposes Mode for both electrical and finical Weighted Average finical purposes Generalized mean for electrical measurement purposes Geometric mean for electrical measurement purposes Harmonic mean for electrical measurement purposes what would be the best way to create these new functions??
On Fri, Feb 29, 2008 at 12:11:59PM -0500, Justin wrote: > Need help and direction creating new aggregate functions. > > We need to add more average functions for both scientific and finical > purposes [ ... ] > what would be the best way to create these new functions?? I'd be tempted to look at pl/r[1], R[2] is a language for statistical analysis and pl/r integrates it into Postgres. Sam [1] http://joeconway.com/plr/[2] http://www.r-project.org/
This probably belongs on General, but On Fri, Feb 29, 2008 at 9:11 AM, Justin <justin@emproshunts.com> wrote: > Need help and direction creating new aggregate functions. > > We need to add more average functions for both scientific and finical > purposes > > RMS for electrical measurement purposes > Mode for both electrical and finical > Weighted Average finical purposes > Generalized mean for electrical measurement purposes > Geometric mean for electrical measurement purposes > Harmonic mean for electrical measurement purposes > > what would be the best way to create these new functions?? Have you already read the documentation on creating aggregates? Have you tried something and it didn't work, or are you interested in design ideas? I would just knock together something in plpgsql. If you have trouble, send the specific questions to the list. "best" ? C is fastest, etc -- what kind of tradeoffs do you need to satisfy? One thing worth thinking about is using arrays to carry state from one function call to the next in an aggregate; this is how the function used in the average aggregate keeps track of both the running total and the number of rows. The Stdev uses a three item array similarly. > > > ---------------------------(end of broadcast)--------------------------- > TIP 6: explain analyze is your friend >