Fix memory leak in BasicCommandStore

[3cky]

Currently scheduled commands are not removed from BasicCommandStore upon executing, so memory leak is introduced. This patch fixes memory leak and also simplifies command scheduling and timeout handling code.

[jbonofre]

R: @jbonofre

[jbonofre]

Your PR breaks command support (a command never get the pending result from the store).

[jbonofre]

I'm checking why.

[jbonofre]

OK, the problem is related to:

commandStore.getPending().remove(id);

It removes the pending command (all commands) from any recipient without waiting the result leading to a timeout.

[jbonofre]

I don't think it's actually a problem. Let me explain.

The ClusteredExecutionContext creates a thread to cleanup the command after timeout (consumed or not). It's where we have the timeoutScheduler (a ExecutorService). The ClusteredExecutionContext systematically creates a TimeoutTask for each Command that cleanup the pending map in the BasicCommandStore. So there is no memory leak IMHO.

[3cky]

Thanks for reviewing this PR, @jbonofre! In fact, we're using forked version of Cellar, so I done code bases comparison and I think this memory leak is already fixed in master by commit 71a5f9e. Still not clear to me why commandStore.getPending().remove(id) breaks command support, seem to work in our case. Nevertheless, this PR can be closed, I think.

[jbonofre]

+1
Can you please close the PR ?

Thanks !