fix issue #41 #43
fix issue #41 #43
Conversation
introduced isResourceAllocated and isError. Support reconnect.
| IbvContext context = endpoint.getIdPriv().getVerbs(); | ||
| RdmaActiveCqProcessor<C> cqProcessor = cqMap.get(context.getCmd_fd()); | ||
| cqProcessor.unregister(endpoint); | ||
| } |
There was a problem hiding this comment.
Why do we allow partially allocated ActiveEndpoints to call close() in the first place, only to then check if they have been allocated and prevent the close from proceeding? Rather I suggest we check the allocation state in the endpoint and decide whether or not to de-allocate.
That being said, I'm not convinced that preventing close on partially allocated enpoints is a good idea. What if I do want to close and cleanup the resources? I can't do so anymore. Also, this leaves partially allocated endpoints in the group, lingering around forever. I think we should leave it to the application to decide whether they want to close or not, then then make sure the close call closes whatever needs to be closed while not closing what doesn't need to.
There was a problem hiding this comment.
My intention is not to prevent closing partial allocated endpoints, but to prevent releasing un-allocated resource when users decide to close the failed ep.
Rather I suggest we check the allocation state in the endpoint and decide whether or not to de-allocate.
This is exactly what I am trying to achieve by introducing isResourceAllocated(). Application ep's close() implementation should be the following pattern:
super.close();
if (isResourceAllocated())
release its own resources...
Check if resources has been successfully allocated before releasing its own resources. super.close() can be called without checking is because current close() of RdmaEndpoint works fine with partial allocated ep, and RdmaActiveEndpoint's needs the patch you quoted above to work.
Also, this leaves partially allocated endpoints in the group, lingering around forever.
This is not the case. Since ep throws exceptions before allocating resources
this ep never registered into cqProcessor. It is also unregistered from group's clientEndpointMap in
Therefore, no residues left after close().
In summary, users have to choices when catching exception on connect(): 1) close the failed ep and 2) try to connect() again. I think the patch can properly preventing releasing un-allocated resources if users decide to take opt-1. And the logic in connect() can handle the opt-2 situation.
introduce isResourceAllocated() and support reconnect.