fix: do not delete session in close method for BatchReadOnlyTransactionImpl

[rajatbhatta]

• remove session.close() from close() method implementation in
  BatchReadOnlyTransactionImpl class.

• add a cleanup() method to BatchReadOnlyTransaction interface
  and give user the control to delete session when the session
  is no longer in use.

• add tests for txn.cleanup() method.

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

• Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
• Ensure the tests and linter pass
• Code coverage does not decrease (if any source code was changed)
• Appropriate docs were updated (if necessary)

Fixes #1651 ☕️

[thiagotnunes]

Wait, won't this break all the existing clients that are relying on the close method to return the session to the pool? (won't this possibly cause a session pool exhaustion for existing clients?)

cc @olavloite

[rajatbhatta]

As BatchReadOnlyTransactionImpl is an AutoCloseable (via MultiUseReadOnlyTransaction->AbstractReadContext->ReadContext->AutoClosable), having it within a try-with-resources construct allows automatic closing of the transaction resource, which leads the underlying session to be released to the session pool. IMO, it should not lead to session pool exhaustion.

Having an overridden close method with just super.close() won't make sense, as the super's close method will anyways be called during automatic closing.

@olavloite : Please do share your viewpoint as well.

[olavloite]

@thiagotnunes No, the BatchClientImpl does not use sessions from the pool. It gets its session in one of two possible ways:

1. It is the client that creates the BatchReadOnlyTransaction: It creates a session here.
2. It is a client that receives a batch transaction id from a different client: It just references the already existing session that was created by the other client here.

So it means that we will create one session that will not (always) be deleted. We choose to do it this way because we do not know how long all clients will need to finish reading, and the session must be kept alive as long as there are clients still reading. The session will eventually be garbage collected by the backend.

[thiagotnunes]

Oh got it, thanks for the explanation!