Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better handling of FdbFuture lifetime and shutdown #48

Open
KrzysFR opened this issue Jan 17, 2015 · 0 comments
Open

Better handling of FdbFuture lifetime and shutdown #48

KrzysFR opened this issue Jan 17, 2015 · 0 comments
Labels
bug native Interop with the native client library
Milestone

Comments

@KrzysFR
Copy link
Member

KrzysFR commented Jan 17, 2015

The current way FdbFuture* handles are mapped to Task's has some issues with race conditions with destroying the handles concurrently while they are executing, especially during mass future extinction (shutdown of a service, assert failed in a unit test, AppDomain being brought down by a test unit runner, ...)

Also, running some profiling on request heavy workloads points to the ConcurrentDictionary<..> used to store the maps handles to Tasks so that future callbacks can retrieve the original Task when they fire. Also, since FdbFuture is generic, there is multiple verison of this ConcurrentDictionary<..>, one for each kind of future (Slice, KeyValuePair<Slice, Slice>[], ...) which is probably not the best way to split them.

We need to change the approach, and have all the futures spawned by a transaction stored inside the transaction itself, so that we can have a safe way to destroy a transaction and make sure that all its futures are done. That way, calling transaction.Dispose() will make sure that everything is cleaned up.

Since a database instance already tracks its transactions, this in turns means that calling database.Dispose() will destroy all transactions AND futures associated.

The last missing piece is to track all opened database instance at the binding level, so that calling Fdb.Stop() would have a deterministic way to wait for ALL futures to complete/be destroyed before stopping the network thread (which is the one who runs the callbacks and can wakeup the FdbFuture objects).

So the shutdown scenario would look like this:

  • set a global flag that forbid any new database instance to be opened
  • ForEach database instance
    • set a global flag that forbid any new transaction to start
    • ForEah transaction instance
      • set a flag to stop any new future from starting
      • ForEach future instance
        • atomically TrySetCancelled and fdb_future_destroy
      • fdb_transaction_destroy
    • fdb_database_destroy (and probably cluster as well)
  • fdb_stop_newtork

Hopefully, when we call fdb_stop_network, all callbacks will either have fired OR we would already have disposed the FdbFuture so that we wouldn't care for the callback to fire.

@KrzysFR KrzysFR added api Issues or changes related to the client API bug and removed api Issues or changes related to the client API labels Jan 17, 2015
@KrzysFR KrzysFR added this to the 0.9.8 milestone Jan 17, 2015
@KrzysFR KrzysFR added the native Interop with the native client library label Jan 17, 2015
@KrzysFR KrzysFR modified the milestones: 0.9.8, 0.9.9 Jan 26, 2015
@KrzysFR KrzysFR changed the title Better hanling of FdbFuture lifetime and shutdown Better handling of FdbFuture lifetime and shutdown Feb 10, 2015
@KrzysFR KrzysFR closed this as completed Apr 1, 2015
@KrzysFR KrzysFR reopened this Apr 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug native Interop with the native client library
Projects
None yet
Development

No branches or pull requests

1 participant