Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OHFJIRA-106] : Circuit breaker #101

Merged
merged 3 commits into from Nov 10, 2019

Conversation

@kkovarik
Copy link
Collaborator

kkovarik commented Jul 10, 2019

Circuit breaker

Documentation : https://openhubframework.atlassian.net/wiki/spaces/OHF/pages/1177288705/circuit
Please let me know what parts of it should be improved in any way, thanks!

Part of this pull request is camel component itself & two implementations of CircuitBreaker interface. Both are mentioned in documentation, one is in-memory, second is hazelcast based. There are three commits, as it may be quite a lot of code for one PR.

Open questions

  • name of the component - should it be circuit or circuit-breaker? currently it is circuit, to avoid boiler plate, as while used it can easily become quite unreadable: circuit:my-system:http4://www.my-system.com/resource?method=POST&timeout=40000...
  • shouldn´t the configuration of component be more "camel-like", meaning it would parse uri and not to use the exchange property?

Possible improvements

  • currently only one implementation of CircuitBreaker interface is active for all circuits, technically it could be modified, that each circuit could have its own
@kkovarik kkovarik added the task label Jul 10, 2019
@kkovarik kkovarik added this to the 2.2 milestone Jul 10, 2019
@kkovarik kkovarik requested review from hanusto and pjuza Jul 10, 2019
@kkovarik kkovarik self-assigned this Jul 10, 2019
@coveralls

This comment has been minimized.

Copy link

coveralls commented Jul 10, 2019

Coverage Status

Coverage increased (+0.9%) to 69.985% when pulling c832765 on feature/OHFJIRA-106-circuit-breaker into e2e9d3c on develop.

* @author Karel Kovarik
* @since 2.2
*/
public class CircuitDownException extends RuntimeException {

This comment has been minimized.

Copy link
@pjuza

pjuza Jul 21, 2019

Contributor

Muze to dedit od IntegrationException?

This comment has been minimized.

Copy link
@kkovarik

kkovarik Sep 9, 2019

Author Collaborator

Tak asi by mohlo. IntegrationException by měly být všechny vyjímky z "vnitřku" OpenHubu? :)

This comment has been minimized.

Copy link
@pjuza

This comment has been minimized.

Copy link
@kkovarik

kkovarik Sep 10, 2019

Author Collaborator

Supr, díky, upraveno :)

static final long serialVersionUID = 1L;

private long lastShortcutTimestamp;
private List<Long> successCallList = new CopyOnWriteArrayList<>();

This comment has been minimized.

Copy link
@pjuza

pjuza Jul 21, 2019

Contributor

Neni to zbytecny komfort pouzivat CopyOnWriteArrayList? Chapu, ze ti to vyresi problem s pristupem pres get() ke kolekci, ale trpi tim performance a mozna zbytecne. Davam k uvaze ...

This comment has been minimized.

Copy link
@kkovarik

kkovarik Sep 9, 2019

Author Collaborator

Dobrá připomínka, vzniklo to při implementaci InMemory varianty. Možná opravdu zbytečné, upravím a doplním komentářem

protected Endpoint createEndpoint(String uri, String remaining, Map<String, Object> parameters) throws Exception {
final CircuitEndpoint endpoint = new CircuitEndpoint(uri, this);

final String endpointURI = ObjectHelper.after(uri, ":");

This comment has been minimized.

Copy link
@pjuza

pjuza Jul 29, 2019

Contributor

IDE me ukazuje, ze ObjectHelper je deprecated, ze mas pouzit StringHelper

This comment has been minimized.

Copy link
@kkovarik

kkovarik Sep 9, 2019

Author Collaborator

upraveno, díky

@@ -0,0 +1,91 @@
package org.openhubframework.openhub.component.circuitbreaker;

This comment has been minimized.

Copy link
@pjuza

pjuza Jul 29, 2019

Contributor

pridal bych sem i do jinych novych baliku packe-info.java

This comment has been minimized.

Copy link
@kkovarik

kkovarik Sep 9, 2019

Author Collaborator

Díky za upozornění, přidal jsem kde jsem našel

// verify configuration is present, otherwise fail as it is misconfigured
final CircuitConfiguration circuitConfiguration = exchange.getProperty(
CircuitBreaker.CONFIGURATION_PROPERTY, CircuitConfiguration.class);
Assert.notNull(circuitConfiguration, "the circuitConfiguration was not set.");

This comment has been minimized.

Copy link
@pjuza

pjuza Jul 29, 2019

Contributor

Asi bych jeste pridal info, ze neni nastaveno v Exchange + nazev te property, protoze preci jen to je trochu nestandardni, takze tato chyba bude vylitavat casto :)

This comment has been minimized.

Copy link
@kkovarik

kkovarik Sep 9, 2019

Author Collaborator

Určitě, dobrý tip 👍
Doplněno.

* see its javadoc.
*
* Usage:
* .doTry()

This comment has been minimized.

Copy link
@pjuza

pjuza Jul 29, 2019

Contributor

chtelo by to nejake formatovace do javaDoc, jinak je pak vse na jednom radku.

This comment has been minimized.

Copy link
@kkovarik

kkovarik Sep 9, 2019

Author Collaborator

upraveno, díky za upozornění

default CircuitConfiguration getCircuitConfiguration(Exchange exchange) {
final CircuitConfiguration circuitConfiguration =
exchange.getProperty(CONFIGURATION_PROPERTY, CircuitConfiguration.class);
Assert.notNull(circuitConfiguration, "the circuitConfiguration was not found");

This comment has been minimized.

Copy link
@pjuza

pjuza Jul 29, 2019

Contributor

tady bych tu chybovou hlasku take rozsiril ...

This comment has been minimized.

Copy link
@kkovarik

kkovarik Sep 9, 2019

Author Collaborator

done

/**
* Unique name of circuit.
*/
private String circuitName;

This comment has been minimized.

Copy link
@pjuza

pjuza Jul 29, 2019

Contributor

zadny konstruktor? mozna jemn nastavit nejaky default circuitName ...

@@ -0,0 +1,211 @@
package org.openhubframework.openhub.core.circuitbreaker;

This comment has been minimized.

Copy link
@pjuza

pjuza Jul 29, 2019

Contributor

pridal bych package-info.java

This comment has been minimized.

Copy link
@kkovarik

kkovarik Sep 9, 2019

Author Collaborator

přidáno

@kkovarik kkovarik force-pushed the feature/OHFJIRA-106-circuit-breaker branch from eeba000 to 279d10b Sep 9, 2019
@kkovarik kkovarik force-pushed the feature/OHFJIRA-106-circuit-breaker branch from 279d10b to c832765 Sep 10, 2019
@pjuza
pjuza approved these changes Sep 11, 2019
Copy link
Contributor

pjuza left a comment

Super, thank you for changes.

Please look at Sonar issues (https://sonarcloud.io/dashboard?id=OpenWiseSolutions_openhub-framework&pullRequest=101) if something is useful for you before final finishing.

@kkovarik kkovarik merged commit 172b7cd into develop Nov 10, 2019
4 checks passed
4 checks passed
SonarCloud Code Analysis Quality Gate passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 69.969%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.