Reenable azure repository tests and lower down max error per request

[tlrx]

This pull request reenables some repository integration tests, decreases the maximum number of failures per request in Azure tests and sets an ExecutorService for the HttpServer used in tests.

I wasn't able to reproduce to failures reported in #47948 and #47380 but I suspect that the highest max. errors per request of 3 is picked up and the fact that requests are processed using the same thread that started the HTTP server might be the cause of the failures. I also think that with more parallelization added in snapshot/restore using an executor service in tests makes sense.

Closes #47948
Closes #47380

[elasticmachine]

Pinging @elastic/es-distributed (:Distributed/Snapshot/Restore)

[tlrx]

@elasticmachine run elasticsearch-ci/1

(Test is green but I'd like to have more runs)

[tlrx]

@elasticmachine run elasticsearch-ci/1

(Test is green but I'd like to have more runs)

[tlrx]

@elasticmachine run elasticsearch-ci/1

(Test is green but I'd like to have more runs)

[original-brownbear]

@tlrx I'm not so sure about this one. Can slowness due to executing the handlers on the HTTP server's callbacks actually lead to EOF exceptions? (to me it seems no, but I haven't worked much with the HttpServer before this) Isn't it more likely we're failing to consume the full input here in some corner case and corrupt the Http/1.1 stream that way?

[original-brownbear]

See my question, I'm a little unsure about the fix, but if the failures are all due to the 3x retry I think we're looking at the right thing probably.

[tlrx]

Well I did not claim it's a fix :)

I tried to reproduce the Azure failures by having the tests run over the weekend but I did not manage to reproduce. Also, by looking at the build stats I've found some GCS failures with read timeouts and assertion errors on server side.

I'd expect any failure related to read timeouts or errors/retries to be reproductible, but actually it is not because of the randomBoolean() (added to make the tests faster) that randomizes things too much and let requests fail or succeed depending on the order they are executed.

Can slowness due to executing the handlers on the HTTP server's callbacks actually lead to EOF exceptions?

Honestly I'm not sure. But the internal HTTP servers are still single threaded and we are adding more parallelization to snapshot/restore so I think we should change that so that it better reflects how storage services are working and it avoid to have one operation to be dependent on how another unrelated operation performs.

but if the failures are all due to the 3x retry I think we're looking at the right thing probably.

That was my initial guess but the fact that the failures do not reproduce easily and that Azure retries test are testing the max. number of retries let me think that it's maybe a factor but maybe not the root cause of the failures. Since Azure executes more requests (HEAD+GET) maybe they are just failing more often than the others.

At this point I agree that this PR is maybe changing too many things at once.

We could maybe do instead:

• remove the randomBoolean() on the server side
• use an executor service on the server side
• reenable azure tests and see how it performs (also monitor S3/GCS)

Depending of the result, revisit the decrease of 3 to 2 max. errors per request. What do you think?

[original-brownbear]

remove the randomBoolean() on the server side

I like this a lot :) Making things more deterministic sounds like the right move.

use an executor service on the server side

I don't like this tbh. I can't see how lowering the latency of the IO thread by forking cheap operations will help us here. Also, even if it does help, then it's a strange spot because it basically means that our tests are broken and just accidentally work on fast boxes?

On this topic:

I think we should change that so that it better reflects how storage services are working and it avoid to have one operation to be dependent on how another unrelated operation performs.

I'm not sure this is a valid point. Even though not using a thread-pool for handling requests does make all the request handling sequential it does not however introduce any artificial ordering as far as I can see. In the same way I argued above, I'd say here: If we fail because of sequential handling of requests, then something must either be seriously wrong with the SDK or with our code mustn't it? :)

reenable azure tests and see how it performs (also monitor S3/GCS)

Jup let's do suggestion 1 (remove the random boolean) and reenable ? :)

[tlrx]

Thanks Armin, I tend to follow your networking expertise :) Still, I'm not sure all operations are that cheap on CI boxes (I'm thinking of few megabytes uploads or multiple uploads/deletions at once that "hold on" connections but do nothing with them).

Jup let's do suggestion 1 (remove the random boolean) and reenable ? :)

Sure, I'll adapt this PR.

[original-brownbear]

Jenkins run elasticsearch-ci/bwc

[original-brownbear]

LGTM thanks Tanguy!

[tlrx]

@elasticmachine run elasticsearch-ci/bwc

[tlrx]

@elasticmachine update branch

[tlrx]

Thanks @original-brownbear.