Re: Solving the OID-collision problem - Mailing list pgsql-hackers

Just to chime in --- I have been surprised how _few_ complaints we have
gotten about oid wraparound hitting system table oid conflicts.  I agree
that telling people to retry their CREATE statements isn't really an
ideal solution, and the idea of looping to find a free oid is a good one.

---------------------------------------------------------------------------

Tom Lane wrote:
> I was reminded again today of the problem that once a database has been
> in existence long enough for the OID counter to wrap around, people will
> get occasional errors due to OID collisions, eg
> 
> http://archives.postgresql.org/pgsql-general/2005-08/msg00172.php
> 
> Getting rid of OID usage in user tables doesn't really do a darn thing
> to fix this.  It may delay wrap of the OID counter, but it doesn't stop
> it; and what's more, when the problem does happen it will be more
> serious (because the OIDs assigned to persistent objects will form a
> more densely packed set, so that you have a greater chance of collisions
> over a shorter time period).
> 
> We've sort of brushed this problem aside in the past by telling people
> they could just retry their transaction ... but why don't we make the
> database do the retrying?  I'm envisioning something like the attached
> quick-hack, which arranges that the pg_class and pg_type rows for tables
> will never be given OIDs duplicating an existing entry.  It basically
> just keeps generating and discarding OIDs until it finds one not in the
> table.  (This will of course not work for user-table OIDs, since we
> don't necessarily have an OID index on them, but it will work for all
> the system catalogs that have OIDs.)
> 
> I seem to recall having thought of this idea before, and having rejected
> it as being too much overhead to solve a problem that occurs only rarely
> --- but in a quick test involving many repetitions of
> 
>     create temp table t1(f1 int, f2 int);
>     drop table t1;
> 
> the net penalty was only about a 2% slowdown on one machine, and no
> measurable difference at all on another.  So it seems like it might
> be worth doing.
> 
> Comments?
> 
>             regards, tom lane
> 
> 
> *** src/backend/catalog/heap.c.orig    Thu Jul 28 16:56:40 2005
> --- src/backend/catalog/heap.c    Wed Aug  3 19:20:22 2005
> ***************
> *** 187,192 ****
> --- 187,229 ----
>    * ---------------------------------------------------------------- */
>   
>   
> + /*
> +  * Quick hack to generate an OID not present in the specified catalog
> +  */
> + static Oid
> + safe_newoid(Oid catalogId, Oid oidIndexId)
> + {
> +     Oid            newOid;
> +     Relation    catalogRelation;
> +     SysScanDesc scan;
> +     ScanKeyData key;
> +     bool        collides;
> + 
> +     catalogRelation = heap_open(catalogId, AccessShareLock);
> + 
> +     do
> +     {
> +         newOid = newoid();
> + 
> +         ScanKeyInit(&key,
> +                     ObjectIdAttributeNumber,
> +                     BTEqualStrategyNumber, F_OIDEQ,
> +                     ObjectIdGetDatum(newOid));
> + 
> +         scan = systable_beginscan(catalogRelation, oidIndexId, true,
> +                                   SnapshotNow, 1, &key);
> + 
> +         collides = HeapTupleIsValid(systable_getnext(scan));
> + 
> +         systable_endscan(scan);
> +     } while (collides);
> + 
> +     heap_close(catalogRelation, AccessShareLock);
> + 
> +     return newOid;
> + }
> + 
> + 
>   /* ----------------------------------------------------------------
>    *        heap_create        - Create an uncataloged heap relation
>    *
> ***************
> *** 227,233 ****
>        * Allocate an OID for the relation, unless we were told what to use.
>        */
>       if (!OidIsValid(relid))
> !         relid = newoid();
>   
>       /*
>        * Decide if we need storage or not, and handle a couple other
> --- 264,270 ----
>        * Allocate an OID for the relation, unless we were told what to use.
>        */
>       if (!OidIsValid(relid))
> !         relid = safe_newoid(RelationRelationId, ClassOidIndexId);
>   
>       /*
>        * Decide if we need storage or not, and handle a couple other
> ***************
> *** 714,720 ****
>       new_rel_oid = RelationGetRelid(new_rel_desc);
>   
>       /* Assign an OID for the relation's tuple type */
> !     new_type_oid = newoid();
>   
>       /*
>        * now create an entry in pg_class for the relation.
> --- 751,757 ----
>       new_rel_oid = RelationGetRelid(new_rel_desc);
>   
>       /* Assign an OID for the relation's tuple type */
> !     new_type_oid = safe_newoid(TypeRelationId, TypeOidIndexId);
>   
>       /*
>        * now create an entry in pg_class for the relation.
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend
> 

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073