search

etcd-io / jetcd

etcd java client
Apache License 2.0
1.08k stars 314 forks source link

Retry auth failures and require leader in ElectionImpl. #1337

Closed jcferretti closed 3 months ago

jcferretti commented 3 months ago

Fixes #1336

Also adds require leader in LockImpl; LockImpl was already using execute, so auth retries were happening there.

jcferretti commented 3 months ago

Perhaps we should do the same for the lock API; my team is not currently using that so I did not think about it at first. You can let me know if you'd like me to do the same changes for lock, I think it would make sense.

lburgazzoli commented 3 months ago

@jcferretti whatever you can do to help the project would be more than welcome since my time, as you have noticed, is very very limited

jcferretti commented 3 months ago

Understood, will do. I will add to this same PR and change the title to reflect the scope. I'll have something to you by tomorrow.

jcferretti commented 3 months ago

Done, PTAL at your convenience.

jcferretti commented 3 months ago

Tests pass locally for me:

cfs@tenten 23:40:08 ~/github/jetcd
$ ./gradlew test
Starting a Gradle Daemon (subsequent builds will be faster)
Java HotSpot(TM) 64-Bit Server VM warning: Sharing is only supported for boot loader classes because bootstrap classpath has been appended

> Task :jetcd-core:test

  io.etcd.jetcd.op.TxnTest ✔ testIfs()

[...]
  io.etcd.jetcd.impl.WatchResumeTest > testWatchOnPut(int) ✔ [5] 60 (1m 10s)

  135 passing (4m 10s)
  12 pending

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.5/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

BUILD SUCCESSFUL in 4m 15s
25 actionable tasks: 3 executed, 22 up-to-date

The failure on the github PR checks is on WatchTest.testWatchOnDeletion:

[...]
  io.etcd.jetcd.impl.WatchTest > testWatchOnDelete(Client) ✔ [1] io.etcd.jetcd.impl.ClientImpl@5987e932
  io.etcd.jetcd.impl.WatchTest > testWatchOnDelete(Client) ✘ [2] io.etcd.jetcd.impl.ClientImpl@a518813 (30s)

    java.util.concurrent.TimeoutException: testWatchOnDelete(io.etcd.jetcd.Client) timed out after 30 seconds

WatchTest > testWatchOnDelete(Client) > [2] io.etcd.jetcd.impl.ClientImpl@a518813 FAILED
    java.util.concurrent.TimeoutException: testWatchOnDelete(io.etcd.jetcd.Client) timed out after 30 seconds
        at org.junit.jupiter.engine.extension.TimeoutExceptionFactory.create(TimeoutExceptionFactory.java:29)
        at org.junit.jupiter.engine.extension.SameThreadTimeoutInvocation.proceed(SameThreadTimeoutInvocation.java:58)
[...]

Not sure how the code in this PR could have broken that one.

lburgazzoli commented 3 months ago

@jcferretti can you squash the commits ? (and sign off the commit if you can)

jcferretti commented 3 months ago

Force-pushed to fix DCO (commit sign-off).