From 557cf1e60e2ab67d43f1ea02c8037078705cc0a6 Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Mon, 29 Jul 2024 10:23:56 +0200 Subject: [PATCH] Add a tombstone operation to optimize multiple deletes Closes #31699 Signed-off-by: Alexander Schwartz --- .../InfinispanKeycloakTransaction.java | 26 ++++++++++++------- .../RemoteInfinispanKeycloakTransaction.java | 24 ++++++++++++++--- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanKeycloakTransaction.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanKeycloakTransaction.java index 177f9092e8..313799a3b6 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanKeycloakTransaction.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanKeycloakTransaction.java @@ -1,13 +1,13 @@ /* * Copyright 2017 Red Hat, Inc. and/or its affiliates * and other contributors as indicated by the @author tags. - * + * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -122,10 +122,12 @@ public class InfinispanKeycloakTransaction implements KeycloakTransaction { Object taskKey = getTaskKey(cache, key); CacheTask current = tasks.get(taskKey); - if (current != null) { + if (current != null && current != TOMBSTONE && current.getOperation() != Operation.REMOVE) { if (current instanceof CacheTaskWithValue) { ((CacheTaskWithValue) current).setValue(value); ((CacheTaskWithValue) current).updateLifespan(lifespan, lifespanUnit); + } else { + throw new IllegalStateException("Can't replace entry: task " + current + " in progress for session"); } } else { tasks.put(taskKey, new CacheTaskWithValue(value, lifespan, lifespanUnit) { @@ -171,6 +173,10 @@ public class InfinispanKeycloakTransaction implements KeycloakTransaction { return String.format("CacheTask: Operation 'remove' for key %s", key); } + @Override + public Operation getOperation() { + return Operation.REMOVE; + } }); } @@ -200,9 +206,13 @@ public class InfinispanKeycloakTransaction implements KeycloakTransaction { public interface CacheTask { void execute(); + + default Operation getOperation() { + return Operation.OTHER; + } } - public enum Operation { PUT, OTHER } + public enum Operation { PUT, REMOVE, OTHER } public static abstract class CacheTaskWithValue implements CacheTask { protected V value; @@ -227,10 +237,6 @@ public class InfinispanKeycloakTransaction implements KeycloakTransaction { this.lifespan = lifespan; this.lifespanUnit = lifespanUnit; } - - public Operation getOperation() { - return Operation.OTHER; - } } // Ignore return values. Should have better performance within cluster / cross-dc env @@ -240,4 +246,4 @@ public class InfinispanKeycloakTransaction implements KeycloakTransaction { return ((Cache) cache).getAdvancedCache() .withFlags(Flag.IGNORE_RETURN_VALUES, Flag.SKIP_REMOTE_LOOKUP); } -} \ No newline at end of file +} diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/remote/RemoteInfinispanKeycloakTransaction.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/remote/RemoteInfinispanKeycloakTransaction.java index 9d442f3ddf..1e32fc1734 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/remote/RemoteInfinispanKeycloakTransaction.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/remote/RemoteInfinispanKeycloakTransaction.java @@ -31,6 +31,7 @@ import io.reactivex.rxjava3.core.Flowable; import org.infinispan.client.hotrod.MetadataValue; import org.infinispan.client.hotrod.RemoteCache; import org.infinispan.commons.util.concurrent.AggregateCompletionStage; +import org.infinispan.commons.util.concurrent.CompletableFutures; import org.infinispan.commons.util.concurrent.CompletionStages; import org.jboss.logging.Logger; import org.keycloak.models.AbstractKeycloakTransaction; @@ -76,7 +77,7 @@ public class RemoteInfinispanKeycloakTransaction extends AbstractKeycloakT logger.tracef("Adding %s.put(%S)", cache.getName(), key); if (tasks.containsKey(key)) { - throw new IllegalStateException("Can't add session: task in progress for session"); + throw new IllegalStateException("Can't add entry: task " + tasks.get(key) + " in progress for session"); } tasks.put(key, new PutOperation<>(key, value, lifespan, timeUnit)); @@ -86,9 +87,11 @@ public class RemoteInfinispanKeycloakTransaction extends AbstractKeycloakT logger.tracef("Adding %s.replace(%S)", cache.getName(), key); Operation existing = tasks.get(key); - if (existing != null) { + if (existing != null && existing != TOMBSTONE && !(existing instanceof RemoteInfinispanKeycloakTransaction.RemoveOperation)) { if (existing.hasValue()) { tasks.put(key, existing.update(value, lifespan, timeUnit)); + } else { + throw new IllegalStateException("Can't replace entry: task " + existing + " in progress for session"); } return; } @@ -101,7 +104,8 @@ public class RemoteInfinispanKeycloakTransaction extends AbstractKeycloakT Operation existing = tasks.get(key); if (existing != null && existing.canRemove()) { - tasks.remove(key); + //noinspection unchecked + tasks.put(key, (Operation) TOMBSTONE); return; } @@ -254,4 +258,18 @@ public class RemoteInfinispanKeycloakTransaction extends AbstractKeycloakT return cache.removeAsync(key); } } + + private static final Operation TOMBSTONE = new Operation<>() { + + @Override + public boolean canRemove() { + return true; + } + + @Override + public CompletionStage execute(RemoteCache cache) { + return CompletableFutures.completedNull(); + } + }; + }