Lease collection requirements

Azure / azure-documentdb-changefeedprocessor-dotnet

This library provides a host for distributing change feed events in partitioned collection across multiple observers. Instances of the host can scale up (by adding) or down (by removing) dynamically, and the load will be automatically distributed among active instances in about-equal way.

Other

51 stars 22 forks source link

Lease collection requirements #94

Closed chwarr closed 5 years ago

chwarr commented 6 years ago

Is there (should there be, if not) documentation that describes the requirements of the lease document collection? Is there a library function to create the collection for me? If not, should there be?

Things I've noticed:

It looks like the lease collection needs to have TTL enabled otherwise the TimeToLive on the initializing lock document will never cause a deletion to happen.
- If the process that holds the initialization lock crashes before completing, the lock will never be deleted. It looks like all subsequent processes will wait for initialization, which will never occur.
The lease collection operations do not specify a partition key, so the lease collection cannot be one that requires a partition key (e.g., in a database with its own provisioned throughput or in an unlimited collection).
- It looks like the lease collection operations could specify a partition key. I can potentially submit a PR to add that, if you're OK with it. Please let me know about this part.

chwarr commented 6 years ago

Checking back in after a couple of days. Do you have an estimate for when someone will be able to look at this issue? Thanks.

chwarr commented 6 years ago

Is there any idea when someone can take a look at this issue?

ealsur commented 6 years ago

@chwarr I'm not aware of any requirement other than the Lease Collection needs to be Single Partition.

Adding TTL could potentially delete leases if the Hosts are (for some reason) turned off longer than the specified TTL, causing changes to be lost between the last known checkpoint and the next time they turn on.

Partition Collection support is considered but I'm not sure what's the roadmap, @mkolt ?

chwarr commented 6 years ago

Just skimmed the lease code. I agree that those documents shouldn't expire. The EqualPartitionsBalancingStrategy uses the _ts field to detect a stale lease and steal it.

The lock document is the only one that appears to have its TimeToLive property set.

So, if the collection had an default TTL of -1, then the lock document would expire based on its explicit TTL. The leases would default to -1 and the documents would never expire.

Alternatively, the lock code could be re-written to check the document's _ts property and explicitly steal a stale lock, like is done with collections.

chwarr commented 6 years ago

Checking in again after a few days.

I still think that the collection is intended to have a TLL of -1 so that initialization lock document expires. Alternatively, the code needs to manually check expiration and retry initialization.

mkolt commented 6 years ago

To answer: 1) There is no need to define TTL on lease collection. The documents in lease collections must not have TTL (well, not yet), the expiration is checked on the client. 2) Lease colleciton currently has to be single partition. Long-term we should support partitioned collections.

chwarr commented 6 years ago

There is no need to define TTL on lease collection. The documents in lease collections must not have TTL (well, not yet), the expiration is checked on the client.

I do not see any code that checks the expiration of the initialization lock document. Can you help point me at the client-side checking for this document?

I did find the code that checks the lease documents expiration time client-side.

Lease colleciton currently has to be single partition. Long-term we should support partitioned collections.

Sounds good. I assume that you're not in a position to take a PR to add this support, so I won't submit one.

mkolt commented 6 years ago

Actually, true, the lock document is using TTL (so you can set collection level TTL to -1), this only affects the case when .info document hasn't been created. That's something I'd like to revisit. Lease documents are not using TTL.

For PR if you want to contribute - you are always welcome :)

mkolt commented 5 years ago

No activity for long time. See this PR https://github.com/Azure/azure-documentdb-changefeedprocessor-dotnet/pull/114 for supporting lock release without TTL.