[ https://issues.apache.org/jira/browse/QPID-7658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15950830#comment-15950830 ]
Lorenz Quack commented on QPID-7658: ------------------------------------ * {{NamedAddressSpace#getSendingLink(String, String)}} and {{NamedAddressSpace#getReceivingLink(String, String)}}: I prefer having separate functions rather than a boolean switch. Furthermore it would not eliminate the if-else-block it would simply move it somewhere else. Therefore I intend not to action this point. * {{AMQPSession#doOnIOThreadAsync}}. Yes this was intentional since there is nothing protocol specific about this and I thought other protocols might find this useful. I also think reducing the differences between the protocol implementations is desirable. No action. * {{LinkImpl#attach}} precondition: No, the condition is correct. The {{_role}} is the local role whereas {{attach.getRole}} returns the remote role. So if the remote peer and the local peer have the same role something is wrong. No action. * I'll remove the {{null}} check and CSRE and modify the {{createLinkEndpoint}} method to ensure it always returns a non-null LinkEndpoint. * You are right, the {{attach}} and link stealing is currently racey. I suggest fixing that as part of QPID-7659 * Regarding removing {{ErrantLinkEndpoint}} in favour of {{hasError}}. I preferred it this way since this is not a generic error reporting facility but only used if the Attach fails to ensure the Link is immediately Detached. However, I'll remove the {{attachWasUnsuccessful}}. It will be the responsibility of the Link#attach to throw an exception in case of an error. * I'll remove the unused variables you mentioned. * I'll rename {{ReceivingLinkEndpoint}} to {{AbstractReceivingLinkEndpoint}} * I'll fix {{SendingLinkEndpoint#remoteDetachedPerformDetach}} * I'll change the error message in {{Session_1_0#receiveAttach}} * Making {{Link_1_0}} and {{LinkEndpoint}} generic is a larger patch. I'll commit that separately. > [Java Broker] Improve LinkRegistry > ---------------------------------- > > Key: QPID-7658 > URL: https://issues.apache.org/jira/browse/QPID-7658 > Project: Qpid > Issue Type: Task > Components: Java Broker > Reporter: Lorenz Quack > Fix For: qpid-java-broker-7.0.0 > > > Currently the AbstractVirtualHost has a Map<remoteContainerId, LinkRegistry>. > The handling of the remoteContainerId should also be encapsulated in the > LinkRegistry giving each VH only a single LinkRegistry. > The LinkRegistry is responsible for ensuring Link uniqueness and persistence > (separate JIRA). > Furthermore, the following change to the LinkRegistry is proposed: > * LinkRegistry.getSendvingLink(String remoteContrainerId, String linkName) -> > Link > * LinkRegistry.getReceivingLink(String remoteContrainerId, String linkName) > -> Link > These should always return a non-null Link. The caller (session) is > responsible for checking that the link has valid Source and Targets. > They also need to be thread-safe (e.g., two calls with the same arguments > should return the same object). > The LinkRegistry should also provide facilities to the Link to remove itself > from the Registry (for example on Link close, or error) and to update the > Termini upon resuming a Link. > If possible this should not be part of the public API. possible API: > * LinkRegistry.removeSendingLink(String localContainerId, String > remoteContrainerId, String linkName) > * LinkRegistry.removeReceivingLink(String localContainerId, String > remoteContrainerId, String linkName) > * LinkRegistry.updateLinkTermini(Link link, Source source, Target target) > The implementation of which must be thread-safe because the LinkRegistry will > be accessed on multiple threads concurrently. -- This message was sent by Atlassian JIRA (v6.3.15#6346) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org