Details
-
Improvement
-
Status: Open
-
Major
-
Resolution: Unresolved
-
None
-
None
-
None
-
None
Description
These issues were revealed while reviewing the XCeiverClientManager, and the clients.
- secure clients are not released properly, so the reference counting does not work with secure clients
- even though we have reference counting for the clients, the cache can evict and remove client instances with active references, as it is not connected with the reference counts
- isUseRatis, getFactor, getType is not really belonging to this class
- acquireClient and acquireClientForRead and release methods of the same kind, seems to be a bad smell, we might separate the two things, especially because reads are using the grpc client while writes are using the ratis client as I know
- pipelines are leaking from the clients themselves, the pipelines are not modified in these code paths, but it should be better if we can hide the pipeline, and don't serve it for the clients, or if we can serve an immutable version
- ContainerProtocolCalls seems to be something that is extracted to a utility class but it may be placed into the client itself, as in all the cases, the client is gathered from the XCeiverClientManager, then given to one of ContainerProtocolCalls' method, which calls the sendCommandAsync on the client which does not seem to be necessary, we can encapsulate all the protobuf message creation, and provide response data from the client.
-ContainerOperationClient acquires the client twice from the XCeiverClientManager in the createContainer call, but releases it only once - we can try to get rid of some of the synchronization by trying to eliminate some of the states in the clients, and the manager, and replace them with some polymorphism.
I will go through this list one by one and will create JIRAs one by one using this one as an umbrella JIRA, so that we can create PRs one by one, or if needed, we can consolidate the whole thing into one PR at the end but review one by one.
