Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retry auth failures and require leader in ElectionImpl. #1337

Merged
merged 1 commit into from
Mar 17, 2024

Conversation

jcferretti
Copy link
Contributor

@jcferretti jcferretti commented Mar 12, 2024

Fixes #1336

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

@jcferretti
Copy link
Contributor Author

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
Copy link
Collaborator

@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
Copy link
Contributor Author

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 jcferretti marked this pull request as draft March 13, 2024 19:00
@jcferretti jcferretti marked this pull request as ready for review March 13, 2024 19:19
@jcferretti
Copy link
Contributor Author

Done, PTAL at your convenience.

@jcferretti
Copy link
Contributor Author

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
Copy link
Collaborator

lburgazzoli commented Mar 15, 2024

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

Signed-off-by: Cristian Ferretti <jcferretti2020@gmail.com>
@jcferretti
Copy link
Contributor Author

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

@lburgazzoli lburgazzoli merged commit 2398ebd into etcd-io:main Mar 17, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Election methods do not do auth retries, and do not require leader
3 participants