[sheepdog] [PATCH v2 3/3] object list cache: reclaim object list cache when receiving a deletion event.

[levin li]

On 07/18/2012 03:17 AM, MORITA Kazutaka wrote:
> At Thu, 12 Jul 2012 09:55:27 +0800,
> levin li wrote:
>>
>> On 2012年07月12日 06:01, MORITA Kazutaka wrote:
>>> At Wed, 11 Jul 2012 14:42:47 +0800,
>>> levin li wrote:
>>>>
>>>> From: levin li <xingke.lwp at taobao.com>
>>>>
>>>> Before reclaiming the cache belonging to the VDI just deleted, we should test
>>>> whether the VDI is exist, because after some node delete it and before the
>>>> notification is sent to all the node, another node may issus a VDI creation
>>>> event and reused the VDI id again, in which case we should reclaim the cached
>>>> entry.
>>>
>>> Should this be explained in the source code where vdi_exist() is
>>> called?
> 
> How do you think about this?
> 

As I said in the commit log, before the notify message arrives at every node,
a new cluster request adding the VDI just deleted may be issued, 
when the notify message arrives, if we try to delete the object list entries
belong to this VDI regardless of whether the VDI exists, it may causes fatal
error when next recovery happens.

> 
>>> I'm still a bit confused why we need both objlist_cache_delete and
>>> objlist_cache_remove.  If no recovery happens during object deletion,
>>> we don't need objlist_cache_delete because objlist_cache_remove
>>> removes the entry.  objlist_cache_delete is necessary for the case
>>> object deletion and recovery happen at the same time to clean up
>>> unhandled entries by objlist_cache_remove.  Is it correct?
>>>
>>
>>
>> Well, if no recovery happens before, objlist_cache_remove() can always
>> delete the entry in object list cache while VDI deletion, but once recovery
>> happens, it's something different.
>>
>> For example, obj A and B stay in node n1 at first, but after a recovery,
>> A may be migrated from n1 to n2, and B stays in n1, but the objlist entry
> 
> Is there any reason we don't remove the objlist entry when we unlink
> the object from the farm working directory?
> 
> diff --git a/sheep/farm/trunk.c b/sheep/farm/trunk.c
> index b45427d..e390a63 100644
> --- a/sheep/farm/trunk.c
> +++ b/sheep/farm/trunk.c
> @@ -297,6 +297,7 @@ int trunk_file_write_recovery(unsigned char *outsha1)
>  				goto out;
>  			}
>  			dprintf("remove file %"PRIx64"\n", entry->raw.oid);
> +			objlist_cache_remove(oid);
>  			put_entry(entry);
>  		}
>  	}
> 
> Recovery algorithm has been changed from before, so probably I'm
> missing something.  Correct me if I'm wrong.
> 

We does call objlist_cache_remove() when deleting a specified object, but
objlist_cache_remove() can only delete the specified objlist entry on local
node, but as I explained before the objlist may not stay on the local node
because of recovery migrating the object without migrating the objlist entry.

> Anyway, the function names (objlist_cache_remove/delete) looks too
> confusing.  IIUC, objlist_cache_remove() is used when removing caches
> after processing SD_OP_READ_PEER requests, and objlist_cache_delete()
> is used to cleanup caches associated with removed objects after vdi
> deletion is finished.  Can we use more descriptive names?
> 

Yes, may be we can rename objlist_cache_delete to objlist_cache_cleanup.

thanks,

levin


> Thanks,
> 
> Kazutaka
>