Skip to content

timeout handling for get instances SA-25112#3

Open
marekzebrowski-dotdata wants to merge 2 commits intomasterfrom
zookeeper-log-times
Open

timeout handling for get instances SA-25112#3
marekzebrowski-dotdata wants to merge 2 commits intomasterfrom
zookeeper-log-times

Conversation

@marekzebrowski-dotdata
Copy link

No description provided.

.flatMap(f => Future.sequence(f).map(_.flatten))
.map(_.map(pathToInstanceId))
.recover(notFoundToEmptySeq[InstanceId])
expectedExecution.onComplete { result:Try[Seq[InstanceId]] =>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this future going to be completed on akka-http timeout?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - it is independent. Akka-http waits for completion of http request for akka.http.server.request-timeout
This future will be completed at unknown point. if it is completed within that timeout, result will be set back to http layer and completes the request. Otherwise, request is completed with timeout, but the Future computation here continues and eventually it should finish, logging the result.


# The initial amount of time to wait between retries.
connection-interval-ms = 3000
connection-interval-ms = 500
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you change it?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in ZookeeperClient we have .retryPolicy(new ExponentialBackoffRetry(config.connectionIntervalMs, config.maxRetries)) - so we start from 3000ms and then exponentially backoff up to max-retries. 3000 ms * (2 ^ 8) is already around 12 minutes - way over our timeout. 500ms brings that time to around 2 minutes max

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants