Modify bookbuyer demo to trip circuit breaker

[Jont828]

Modify the bookbuyer demo to spin up several concurrent goroutines that buy books (configured with an env variable) and apply the CRD to see changes in Grafana and the logs.

[draychev]

It would be nice to move this out into an env var (os.Getenv) or a CLI param to the bookbuyer binary -- default it to 1

[draychev]

This is actually interesting - if you merge it like this (before you introduce os.Getenv we'd probably have lower flakyness, which TBH might be a good thing at this stage of the project.
This would run 15 parallel tests. One of them succeeding and notifying ci/maestro that it succeeded would be sufficient for the entire test to be successful (even if the rest of the tests / goroutines fail).

[shashankram]

I would suggest making the env change so that the tests are reliable with defaults. Backpressure is experimental and not enabled by default, so there shouldn't be a need to make concurrent connections. This might have an impact on how the output is rendered, and how success/failures are handled for individual goroutines by maestro.go.

One of them succeeding and notifying ci/maestro that it succeeded would be sufficient for the entire test to be successful (even if the rest of the tests / goroutines fail).

This is not true, if there are any failures those should be caught because it shouldn't fail at all when the backpressure policy is not applied.

[Jont828]

What do you mean by flakiness?

[draychev]

This looks good @Jont828 - you could convert to a ready for review PR.
This PR may be relevant to what you are working on at the moment: #1187 (I added latency and noise to the response time of Bookstore)

[shashankram]

To introduce this feature with minimal impact on the CI, please introduce an env variable to configure numConnections. This will give us better control to fine tune the CI if things go wrong. When backpressure testing is integrated as a part of the CI suite, we can increase numConnections and use that as the default.

[shashankram]

I would suggest making the env change so that the tests are reliable with defaults. Backpressure is experimental and not enabled by default, so there shouldn't be a need to make concurrent connections. This might have an impact on how the output is rendered, and how success/failures are handled for individual goroutines by maestro.go.

One of them succeeding and notifying ci/maestro that it succeeded would be sufficient for the entire test to be successful (even if the rest of the tests / goroutines fail).

This is not true, if there are any failures those should be caught because it shouldn't fail at all when the backpressure policy is not applied.

[Jont828]

@shashankram Would it cause issues to have numConnections=1 as the default so there is only one connection? Or should we revert the default behavior to a non-concurrent bookbuyer?

[shashankram]

@shashankram Would it cause issues to have numConnections=1 as the default so there is only one connection? Or should we revert the default behavior to a non-concurrent bookbuyer?

@shashankram Would it cause issues to have numConnections=1 as the default so there is only one connection? Or should we revert the default behavior to a non-concurrent bookbuyer?

I think keeping it concurrent while leaving the default connection as 1 should be okay because of wg.Wait() should keep the client from not exiting. Existing code looks good, but would leave the default as 1, with a CI_CLIENT_CONCURRENT_CONNECTIONS env variable to make it configurable and only enable it when backpressure is enabled and integrated with the CI.

[draychev]

This looks great - we will continue to iterate!

[draychev]

ref #1376