Page MenuHomePhabricator

[grpc_clients] bump gRPC connection timeout to 30 seconds
ClosedPublic

Authored by varun on Sep 16 2024, 3:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Oct 25, 11:30 PM
Unknown Object (File)
Fri, Oct 25, 12:52 AM
Unknown Object (File)
Tue, Oct 22, 6:30 AM
Unknown Object (File)
Sun, Oct 20, 3:25 AM
Unknown Object (File)
Sat, Oct 19, 3:33 AM
Unknown Object (File)
Sat, Oct 19, 1:35 AM
Unknown Object (File)
Fri, Oct 18, 10:29 PM
Unknown Object (File)
Fri, Oct 18, 10:29 PM
Subscribers

Details

Summary

resolves https://linear.app/comm/issue/ENG-9300/bump-identity-service-timeouts

users with bad internet can sometimes see their client time out while talking to identity. it seems that 5 seconds was too short, so bumping to 30 seconds.

Test Plan

tested this in D10329. also tested the native app with increased timeout to confirm there are no regressions

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun requested review of this revision.Sep 16 2024, 4:14 PM
This revision is now accepted and ready to land.Sep 16 2024, 4:16 PM

Does this affect both native and web?

I think it might be better to bump to something higher. Would there be any downside to 20s or 30s?

Does this affect both native and web?

I think it might be better to bump to something higher. Would there be any downside to 20s or 30s?

Just native and keyserver. The only potential downside is that 20s or 30s may feel unresponsive to users.

add a request timeout of 30 seconds

varun requested review of this revision.Sep 17 2024, 8:51 AM
varun added 1 blocking reviewer(s): ashoat.

30 seconds feels excessive for establishing the tcp connection, but i think it makes sense for completing the actual request

setting @ashoat as blocking

nvm. applying @ashoat 's feedback as intended and deferring the request timeout work

undo request timeout addition

varun retitled this revision from [grpc_clients] bump gRPC connection timeout to 10 seconds to [grpc_clients] bump gRPC connection timeout to 30 seconds.
varun edited the summary of this revision. (Show Details)

fix title and description

This revision was not accepted when it landed; it landed in state Needs Review.Sep 17 2024, 8:59 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.