[ https://issues.apache.org/jira/browse/CURATOR-533?focusedWorklogId=288469&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-288469 ]
ASF GitHub Bot logged work on CURATOR-533: ------------------------------------------ Author: ASF GitHub Bot Created on: 03/Aug/19 21:11 Start Date: 03/Aug/19 21:11 Worklog Time Spent: 10m Work Description: shayshim commented on pull request #320: [CURATOR-533] - improve circuit breaking behavior URL: https://github.com/apache/curator/pull/320#discussion_r310363023 ########## File path: curator-framework/src/main/java/org/apache/curator/framework/state/ConnectionStateListener.java ########## @@ -31,14 +31,14 @@ void stateChanged(CuratorFramework client, ConnectionState newState); /** - * Normally, ConnectionStateListeners are decorated via the configured - * {@link org.apache.curator.framework.state.ConnectionStateListenerDecorator}. For certain - * critical cases, however, this is not desired. If your listener returns <code>true</code> - * for doNotDecorate(), it will not be passed through the decorator. + * ConnectionStateListener managers set via {@link org.apache.curator.framework.CuratorFrameworkFactory.Builder#connectionStateListenerManagerFactory(ConnectionStateListenerManagerFactory)} + * are allowed to proxy (etc.) ConnectionStateListeners as needed. If this method returns <code>true</code> + * the ConnectionStateListener manager must <em>not</em> proxy the listener as it's a vital internal + * listener used by Curator. * * @return true/false */ - default boolean doNotDecorate() + default boolean doNotProxy() Review comment: I think proxy is a bit vague. Why not something like useCircuitBreaker() instead of doNotProxy(), to be more clear about the context it is used in? Unless you see future possibilities to call it also for other 'proxies' than the circuit breaker. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking ------------------- Worklog Id: (was: 288469) Time Spent: 20m (was: 10m) > Improve CURATOR-505 by making the CircuitBreaker instance shared > ---------------------------------------------------------------- > > Key: CURATOR-533 > URL: https://issues.apache.org/jira/browse/CURATOR-533 > Project: Apache Curator > Issue Type: Improvement > Components: Framework, Recipes > Affects Versions: 4.2.0 > Reporter: Jordan Zimmerman > Assignee: Jordan Zimmerman > Priority: Major > Fix For: 4.2.1 > > Time Spent: 20m > Remaining Estimate: 0h > > CURATOR-505 introduced circuit breaking behavior via > {{CircuitBreakingConnectionStateListener}} and > {{ConnectionStateListenerDecorator}}. Elastic has been using it to success > but reports that the implementation can be improved. The existing > implementation uses a new {{CircuitBreaker}} for each > {{ConnectionStateListener}} set in a Curator client. It turns out that this > is not ideal. Instead, a shared {{CircuitBreaker}} should be used per Curator > client. > Unfortunately, the best way to do this is to remove the > {{ConnectionStateListenerDecorator}} semantics and use a different mechanism. > This Issue proposes to do this and remove > {{ConnectionStateListenerDecorator}}. This is a breaking change but given the > short amount of time it's been in Curator it's unlikely that it's been widely > adopted. > If the community considers a breaking change too harsh the older classes can > be maintained for a while and marked as {{@Deprecated}}. -- This message was sent by Atlassian JIRA (v7.6.14#76016)