RE: bzr push into mysql-6.0-falcon-team branch (cpowers:2851 to 2853) Bug#39846

[Vladislav Vaintroub]

Chris,
a tiny nag:

This comment is more irritating than helpful

> -	sync.lock(Shared);
> +	sync.lock(Exclusive); // was Shared

"was Shared" can be easily seen in bzr visualize/ bzr gannotate,  there is
no reason to document it explicitly in source.

> +	Sync sync (&syncObject, "Table::garbageCollect(1)");
> +	sync.lock(Shared);
> +
> +	Sync syncPrior(getSyncPrior(leaving ? leaving : staying),
> "Table::garbageCollect(2)");

I recall , Kevin made an attempt to get away from "lockLocation(1)" or
"lockLocation(2)". The problem with them is when code is changed one need to
reinvent own schema like "1.5" for something inserted between 1 and 2 and
maybe even 1.25,  1.125, 1.0625 if more locks are required. I think the
current unwritten style is like

Sync sync (&syncObject, "Table::garbageCollect(),syncObject"); 

Or  similar, pls check with Kevin

Thanks,
Vlad



> -----Original Message-----
> From: Christopher Powers [mailto:cpowers@stripped]
> Sent: Saturday, October 04, 2008 2:15 AM
> To: commits@stripped
> Subject: bzr push into mysql-6.0-falcon-team branch (cpowers:2851 to
> 2853) Bug#39846
> 
>  2853 Christopher Powers	2008-10-03
>       Bug#39846, "Falcon: Concurrent online alter causes assertion in
> SRLUpdateIndex::append()"
> modified:
>   storage/falcon/DeferredIndex.cpp
>   storage/falcon/SRLUpdateIndex.cpp
> 
>  2852 Christopher Powers	2008-10-03
>       Bug#39845, "Falcon: Concurrent online DROP INDEX causes
> assertion"
> modified:
>   storage/falcon/Table.cpp
> 
>  2851 Christopher Powers	2008-10-03
>       Bug#39795 "Falcon: Online add index does not support index with
> non-null columns"
> 
>       Lift restriction on adding online indexes having non-null
> columns.
> modified:
>   mysql-test/suite/falcon/r/falcon_online_index.result
>   mysql-test/suite/falcon/t/falcon_online_index.test
>   storage/falcon/ha_falcon.cpp
> 
> === modified file 'storage/falcon/DeferredIndex.cpp'
> --- a/storage/falcon/DeferredIndex.cpp	2008-10-02 23:20:47 +0000
> +++ b/storage/falcon/DeferredIndex.cpp	2008-10-04 00:10:34 +0000
> @@ -819,7 +819,7 @@ void DeferredIndex::scanIndex(IndexKey *
>  void DeferredIndex::detachIndex(void)
>  {
>  	Sync sync(&syncObject, "DeferredIndex::detachIndex");
> -	sync.lock(Shared);
> +	sync.lock(Exclusive); // was Shared
>  	index = NULL;
>  }
> 
> 
> === modified file 'storage/falcon/SRLUpdateIndex.cpp'
> --- a/storage/falcon/SRLUpdateIndex.cpp	2008-07-15 18:57:27 +0000
> +++ b/storage/falcon/SRLUpdateIndex.cpp	2008-10-04 00:10:34 +0000
> @@ -40,14 +40,29 @@ SRLUpdateIndex::~SRLUpdateIndex(void)
> 
>  void SRLUpdateIndex::append(DeferredIndex* deferredIndex)
>  {
> +	uint indexId;
> +	int idxVersion;
> +	int tableSpaceId;
> +
> +	Sync syncDI(&deferredIndex->syncObject,
> "SRLUpdateIndex::append");
> +	syncDI.lock(Shared);
> +
> +	if (!deferredIndex->index)
> +		return;
> +	else
> +		{
> +		indexId = deferredIndex->index->indexId;
> +		idxVersion = deferredIndex->index->indexVersion;
> +		tableSpaceId = deferredIndex->index->dbb->tableSpaceId;
> +		}
> +
> +	syncDI.unlock();
> +
>  	Sync syncIndexes(&log->syncIndexes, "SRLUpdateIndex::append(1)");
>  	syncIndexes.lock(Shared);
> 
>  	Transaction *transaction = deferredIndex->transaction;
>  	DeferredIndexWalker walker(deferredIndex, NULL);
> -	uint indexId = deferredIndex->index->indexId;
> -	int idxVersion = deferredIndex->index->indexVersion;
> -	int tableSpaceId = deferredIndex->index->dbb->tableSpaceId;
>  	uint64 virtualOffset = 0;
>  	uint64 virtualOffsetAtEnd = 0;
> 
> 
> === modified file 'storage/falcon/Table.cpp'
> --- a/storage/falcon/Table.cpp	2008-09-08 11:51:19 +0000
> +++ b/storage/falcon/Table.cpp	2008-10-03 23:56:24 +0000
> @@ -2117,7 +2117,10 @@ void Table::garbageCollect(Record *leavi
>  	if (!leaving && !staying)
>  		return;
> 
> -	Sync syncPrior(getSyncPrior(leaving ? leaving : staying),
> "Table::garbageCollect");
> +	Sync sync (&syncObject, "Table::garbageCollect(1)");
> +	sync.lock(Shared);
> +
> +	Sync syncPrior(getSyncPrior(leaving ? leaving : staying),
> "Table::garbageCollect(2)");
>  	syncPrior.lock(Shared);
> 
>  	// Clean up field indexes
> 
> 
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:
> http://lists.mysql.com/commits?unsub=1