This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new fdb03ed448 [#13964] fix(idp-basic): Make service-admin bootstrap
idempotent under races (#13965)
fdb03ed448 is described below
commit fdb03ed44844b636f3d77a5652c968e98e36c20a
Author: MaSai <[email protected]>
AuthorDate: Tue Oct 20 15:04:35 2026 +0800
[#13964] fix(idp-basic): Make service-admin bootstrap idempotent under
races (#13965)
### What changes were proposed in this pull request?
Make `IdpUserGroupManager.initializeConfiguredServiceAdmins` idempotent
under multi-node races: keep the existence pre-check, and treat
`AlreadyExistsException` on insert as success when another node already
created the same configured service admin.
- Only the bootstrap insert path is changed; `addUser` / normal create
APIs still surface already-exists errors.
- Add a concurrent bootstrap unit test.
### Why are the changes needed?
When multiple Gravitino nodes start together, each may observe a missing
service admin and attempt insert. One wins; the other hits the unique
constraint and previously failed Jetty/`gravitino-web` startup with
`AlreadyExistsException`.
Fix: #13964
### Does this PR introduce _any_ user-facing change?
No. Bootstrap becomes more tolerant of concurrent startups; user-facing
IdP create APIs are unchanged.
### How was this patch tested?
- `./gradlew :plugins:idp-basic:test --tests
org.apache.gravitino.idp.TestIdpUserGroupManager -PskipITs`
Co-authored-by: Cursor <[email protected]>
---
.../apache/gravitino/idp/IdpUserGroupManager.java | 15 ++++++++-
.../gravitino/idp/TestIdpUserGroupManager.java | 37 ++++++++++++++++++++++
2 files changed, 51 insertions(+), 1 deletion(-)
diff --git
a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/IdpUserGroupManager.java
b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/IdpUserGroupManager.java
index 0e5fb53fbb..0090f79fc3 100644
---
a/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/IdpUserGroupManager.java
+++
b/plugins/idp-basic/src/main/java/org/apache/gravitino/idp/IdpUserGroupManager.java
@@ -29,6 +29,7 @@ import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.Config;
import org.apache.gravitino.Configs;
import org.apache.gravitino.auth.AuthConstants;
+import org.apache.gravitino.exceptions.AlreadyExistsException;
import org.apache.gravitino.exceptions.NotFoundException;
import org.apache.gravitino.exceptions.UnauthorizedException;
import org.apache.gravitino.idp.basic.IdpCredentialValidator;
@@ -45,6 +46,8 @@ import
org.apache.gravitino.idp.storage.service.IdpGroupMetaService;
import org.apache.gravitino.idp.storage.service.IdpUserMetaService;
import org.apache.gravitino.storage.IdGenerator;
import org.apache.gravitino.storage.relational.utils.POConverters;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Manager for built-in IdP users and groups. It mirrors {@link
@@ -52,6 +55,8 @@ import
org.apache.gravitino.storage.relational.utils.POConverters;
*/
public class IdpUserGroupManager implements Closeable {
+ private static final Logger LOG =
LoggerFactory.getLogger(IdpUserGroupManager.class);
+
private static final IdpUserMetaService USER_SERVICE =
IdpUserMetaService.getInstance();
private static final IdpGroupMetaService GROUP_SERVICE =
IdpGroupMetaService.getInstance();
@@ -116,7 +121,15 @@ public class IdpUserGroupManager implements Closeable {
IdpCredentialValidator.validatePassword(initialAdminPassword);
String passwordHash = passwordHasher.hash(initialAdminPassword);
for (String serviceAdmin : missingServiceAdmins) {
- USER_SERVICE.insertIdpUser(newUserPO(serviceAdmin, passwordHash, true));
+ try {
+ USER_SERVICE.insertIdpUser(newUserPO(serviceAdmin, passwordHash,
true));
+ } catch (AlreadyExistsException e) {
+ // Another node may have created the same service admin between the
existence check and
+ // insert. Treat the unique-constraint conflict as success so
bootstrap stays idempotent.
+ LOG.info(
+ "Configured service admin {} already exists; skipping create
during bootstrap",
+ serviceAdmin);
+ }
}
}
diff --git
a/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/TestIdpUserGroupManager.java
b/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/TestIdpUserGroupManager.java
index 1afa1b5e14..81457fb454 100644
---
a/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/TestIdpUserGroupManager.java
+++
b/plugins/idp-basic/src/test/java/org/apache/gravitino/idp/TestIdpUserGroupManager.java
@@ -35,7 +35,14 @@ import com.google.common.collect.Lists;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
+import java.util.ArrayList;
import java.util.Comparator;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;
import org.apache.gravitino.Config;
import org.apache.gravitino.auth.AuthConstants;
@@ -386,6 +393,36 @@ public class TestIdpUserGroupManager {
"initAdminSame2", manager.authenticate("initAdminSame2",
VALID_PASSWORD).name());
}
+ @Test
+ public void
testInitializeConfiguredServiceAdminsIgnoresConcurrentCreateRace() throws
Exception {
+ loadServiceAdminConfig(BASIC_AUTHENTICATOR, "initAdminRace1");
+
+ int threads = 8;
+ ExecutorService executor = Executors.newFixedThreadPool(threads);
+ CountDownLatch start = new CountDownLatch(1);
+ List<Future<?>> futures = new ArrayList<>();
+ try {
+ for (int i = 0; i < threads; i++) {
+ futures.add(
+ executor.submit(
+ () -> {
+ start.await();
+ manager.initializeConfiguredServiceAdmins(config,
VALID_PASSWORD);
+ return null;
+ }));
+ }
+ start.countDown();
+ for (Future<?> future : futures) {
+ future.get(30, TimeUnit.SECONDS);
+ }
+ } finally {
+ executor.shutdownNow();
+ }
+
+ Assertions.assertEquals(
+ "initAdminRace1", manager.authenticate("initAdminRace1",
VALID_PASSWORD).name());
+ }
+
private static Config createH2Config(Path h2Path) {
Config backendConfig = new Config(false) {};
backendConfig.set(ENTITY_STORE, RELATIONAL_ENTITY_STORE);
