Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#2449 closed defect (fixed)

infinite loop around gserialized_gist_picksplit_2d

Reported by: j3d Owned by: pramsey
Priority: medium Milestone: PostGIS 2.1.1
Component: postgis Version: 2.1.x
Keywords: Cc: aekorotkov@…

Description

it's the second time I came around this issue where the osm2pgsql import hang for a few hours so I tried to figure out whats goind on: perf showed that all usage where in gserialized_gist_picksplit_2d. gdb showed that leftUpper is NaN. I would guess having a bbox with invalid coordinates might cause the issue:

g_box_consider_split (maxLeftCount=150, leftUpper=nan(0x62fb00), minLeftCount≤optimized out>, rightLower=7175427, dimNum=1, context=0x7fffb07e8d80)

Attachments (1)

postgis-picksplit-nan.patch (3.8 KB ) - added by smagen 11 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 by robe, 11 years ago

can you post what versions you are running of both postgis and postgresql using below statement.

SELECT postgis_full_verion() || ' ' || version();

comment:2 by j3d, 11 years ago

"POSTGIS="2.2.0dev r11789" GEOS="3.4.0dev-CAPI-1.8.0 r3829" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.11dev, released 2013/04/13" LIBXML="2.8.0" LIBJSON="UNKNOWN" (core procs from "2.2.0dev r11636" need upgrade) RASTER (raster procs from "2.2.0dev r11636" need upgrade) PostgreSQL 9.2.4 on x86_64-unknown-linux-gnu, compiled by gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3, 64-bit"

comment:3 by j3d, 11 years ago

I forgot to mention that after killing the import (COPY) statement and recreating the gist index further imports (of the same changesets) does work again

comment:4 by pramsey, 11 years ago

Good catch. Presumably if you change

#define KOROTKOV_SPLIT 1

to 0, everything will work, since the picksplit is net new code.

comment:5 by pramsey, 11 years ago

Cc: aekorotkov@… added
Milestone: PostGIS 2.1.1
Version: trunk2.1.x

comment:6 by smagen, 11 years ago

I've reproduced same bug on PostgreSQL core opclasses. Will be fixed.

comment:7 by smagen, 11 years ago

Bug fix for PostgreSQL core: http://www.postgresql.org/message-id/CAPpHfdsRWLoKeJDM8NiX-ChxqAdGw1PQJ7JEhyKEc32+B9UXZA@mail.gmail.com If it will meet no objections from community it will create same patch for PostGIS.

by smagen, 11 years ago

Attachment: postgis-picksplit-nan.patch added

comment:8 by smagen, 11 years ago

Reaction in postgresql-hackers is delayed. But I think patch is OK. Version of patch for PostGIS is attached.

comment:9 by robe, 11 years ago

pramsey - see issue with accepting this patch. Seems like an important one to get thru. I assume trunk will need to be patched as well?

comment:10 by pramsey, 11 years ago

Was hoping to add a reproducable test to the fix commit, but not having any luck locking up my computer with a NaN-salted index operation. Tried this, what should I alter?

create table idxlooptest ( g geometry );
insert into idxlooptest select st_makepoint(a, a) from generate_series(0,10000) a;
-- POINT(1 nan)
insert into idxlooptest select '0101000000000000000000FF7F000000000000FF7F'::geometry from generate_series(0,100);
create index idxlooptest_gix on idxlooptest using gist (g);

comment:11 by pramsey, 11 years ago

Also tried this from the list, and it ran through to completion:

create table test1 as (select st_makepoint(random(), random()) as p from
generate_series(1,1000000));
create index test1_idx on test1 using gist(p);
create table temp as (select * from test1);
insert into temp (select st_makepoint('nan'::float8, 'nan'::float8) from
generate_series(1,1000));
insert into temp (select st_makepoint('nan'::float8, random()) from
generate_series(1,1000));
insert into temp (select st_makepoint(random(), 'nan'::float8) from
generate_series(1,1000));
create table test2 as (select * from temp order by random());
create index test2_idx on test2 using gist(p);
drop table temp;

comment:12 by pramsey, 11 years ago

#2498 provides a usable test case

comment:13 by pramsey, 11 years ago

Resolution: fixed
Status: newclosed

Applied to 2.1 branch at r12008

Applied to trunk at r12009

comment:14 by strk, 11 years ago

Is 2.0 also affected ? Sounds like something which wouldn't hurt backporting if so

comment:15 by robe, 11 years ago

I don't think 2.0 is affected. I think this new gist picksplit algorithm was introduced in 2.1

comment:16 by pramsey, 11 years ago

Correct, this is 2.1+ only

comment:17 by smagen, 11 years ago

I found that there is still low probability of assertion trigger. Assert(lower ≥ context.rightLower); should be replaced with

Assert(lower ≥ context.rightLower
isnan(lower));

Could you commit this small fix>

comment:18 by smagen, 11 years ago

I mean replace

Assert(lower >= context.rightLower);

with

Assert(lower >= context.rightLower || isnan(lower));
Note: See TracTickets for help on using tickets.