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