Allow configuration of read/connect timeout in LDAP client#10925
Allow configuration of read/connect timeout in LDAP client#10925Praveen2112 merged 1 commit intotrinodb:masterfrom
Conversation
Praveen2112
force-pushed
the
praveen/with_custom_config
branch
2 times, most recently
from
9bec775 to
64ba2c0
Compare
Praveen2112
force-pushed
the
praveen/with_custom_config
branch
4 times, most recently
from
7c7049c to
637f826
Compare
Praveen2112
requested review from
aczajkowski,
kokosing,
ksobolew and
mosabua
There was a problem hiding this comment.
Is there a way to control how long the server gets to respond? Otherwise it looks like it's based on luck
There was a problem hiding this comment.
Actually we cannot configure the server to provide a timeout.
Otherwise it looks like it's based on luck
True, but ran it around 1k times it wasn't failing
There was a problem hiding this comment.
If we see flakiness due to this - we could use toxyproxy to increase the latency for queries.
There was a problem hiding this comment.
I would prefer preventing flakiness rather than fixing, though :)
There was a problem hiding this comment.
Well in case of connection timeout we can just create some artificial service that will open socket (ServerSocket should do) but will never respond. That will cause connection timeout for 100%.
Same for read timeout we just need to response with welcome package and afterwards nothing.
There was a problem hiding this comment.
@aczajkowski Have used ToxiProxy which adds latency for these tests thereby failing due to read and connect timeouts.
There was a problem hiding this comment.
Ok makes the job. Java server socket will be just more lightweight. But will be possibly hard to emulate ldap welcome response (no idea to be honest)
|
@ksobolew Have used ToxiProxy to make sure there is a latency, now we could have a way to ensure that connect timeout happens always. |
There was a problem hiding this comment.
Well in case of connection timeout we can just create some artificial service that will open socket (ServerSocket should do) but will never respond. That will cause connection timeout for 100%.
Same for read timeout we just need to response with welcome package and afterwards nothing.
...ssword-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapAuthenticator.java
Outdated
Show resolved
Hide resolved
...in/trino-password-authenticators/src/main/java/io/trino/plugin/password/ldap/LdapConfig.java
Outdated
Show resolved
Hide resolved
...ssword-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapAuthenticator.java
Outdated
Show resolved
Hide resolved
...ssword-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapAuthenticator.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
There is no difference as in tests above. As I understood you before authentication error is raised on connection timeout. Here I was expecting some timeout exception.
There was a problem hiding this comment.
But LDAPClient doesn't throw much of a granular exception, so I guess it would throw something similar to Authentication error
Praveen2112
force-pushed
the
praveen/with_custom_config
branch
from
021a641 to
3be01cc
Compare
|
@kokosing AC |
Praveen2112
force-pushed
the
praveen/with_custom_config
branch
from
3be01cc to
57e4bb0
Compare
Praveen2112
force-pushed
the
praveen/with_custom_config
branch
from
57e4bb0 to
10158c9
Compare
5 participants
No description provided.