[
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)
