Update circuit_breaker.asciidoc

[111andre111]

Tested http://localhost:9200/_nodes/stats/breaker
and found out that the in flight requests are divided with underscores.
This is wrong some versions already.

[danielmitterdorfer]

Thanks for your PR but I think there is a misconception. What you've changed is the circuit breaker setting for the limit and if you check the source code the documentation is already correct:

elasticsearch/server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java

Line 90 in ffc0b9b

Setting.memorySizeSetting("network.breaker.inflight_requests.limit", "100%", Property.Dynamic, Property.NodeScope);

The output in the node stats API is defined by this:

elasticsearch/server/src/main/java/org/elasticsearch/common/breaker/CircuitBreaker.java

Line 56 in ffc0b9b

String IN_FLIGHT_REQUESTS = "in_flight_requests";

I agree that it would be good if those two would be aligned but as this is a breaking change we can only introduce it in the next major version.

As such I think this PR can be closed without merging but would you be up to create a PR for the change that I've proposed (note that this requires documenting it also in the breaking changes docs for 8.0.0).

[111andre111]

Hi Daniel, I will have to dig into what you wrote a little bit. So please give me time for that.

…

-- André Letterer, elastic.co Support Engineer,

On 3. Apr 2019, at 10:45, Daniel Mitterdorfer ***@***.***> wrote: Thanks for your PR but I think there is a misconception. What you've changed is the circuit breaker setting for the limit and if you check the source code the documentation is already correct: https://github.com/elastic/elasticsearch/blob/ffc0b9bb170d6e887444d08d4bc1733de9e6f87e/server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java#L90 <https://github.com/elastic/elasticsearch/blob/ffc0b9bb170d6e887444d08d4bc1733de9e6f87e/server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java#L90> The output in the node stats API is defined by this: https://github.com/elastic/elasticsearch/blob/ffc0b9bb170d6e887444d08d4bc1733de9e6f87e/server/src/main/java/org/elasticsearch/common/breaker/CircuitBreaker.java#L56 <https://github.com/elastic/elasticsearch/blob/ffc0b9bb170d6e887444d08d4bc1733de9e6f87e/server/src/main/java/org/elasticsearch/common/breaker/CircuitBreaker.java#L56> I agree that it would be good if those two would be aligned but as this is a breaking change we can only introduce it in the next major version. As such I think this PR can be closed without merging but would you be up to create a PR for the change that I've proposed (note that this requires documenting it also in the breaking changes docs for 8.0.0). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#40755 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/At5xYfhRUNzYE-ZbONpaaamORMXrruAIks5vdGoygaJpZM4cZNdw>.

[danielmitterdorfer]

Of course. Just ping if you have additional questions.

[111andre111]

Hi Daniel, After reading your issue I got you point. But the question is what should be the leading expression? inflight_requests or in_flight_requests ? -- André Letterer, elastic.co Support Engineer,

…

On 3. Apr 2019, at 11:08, Daniel Mitterdorfer ***@***.***> wrote: Of course. Just ping if you have additional questions. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#40755 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/At5xYQfTaODQiy4uFyEoSIIB4-1Wa11Iks5vdG-ogaJpZM4cZNdw>.

[danielmitterdorfer]

IMHO the less invasive change is to settle on inflight_requests as the name. This only changes the name in log output and node stats output but avoids breaking all related settings (it is still a breaking change but the scope is smaller). Does this make sense?

[111andre111]

Yes, absolutely. This is what I thought as well. -- André Letterer, elastic.co Support Engineer,

…

On 4. Apr 2019, at 11:22, Daniel Mitterdorfer ***@***.***> wrote: IMHO the less invasive change is to settle on inflight_requests as the name. This only changes the name in log output and node stats output but avoids breaking all related settings (it is still a breaking change but the scope is smaller). Does this make sense? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#40755 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/At5xYdlYFt5dwpC-irgpyA6JaIeYbx90ks5vdcRxgaJpZM4cZNdw>.