Re: Solving the OID-collision problem

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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073