From 6ab2bae0be163d3f0b82f619ddab2bed060e9e81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Novotn=C3=BD?= Date: Fri, 25 Oct 2024 16:07:46 +0200 Subject: [PATCH] fix(#717): Client and server timeouts are not aligned Services with deadline MUST NOT be cached and reused. The deadline is calculated during creation. --- .idea/compiler.xml | 35 +------- .../java/io/evitadb/driver/EvitaClient.java | 6 +- .../evitadb/driver/EvitaClientManagement.java | 12 +-- .../io/evitadb/driver/EvitaClientSession.java | 12 +-- .../io/evitadb/driver/StubTimeoutProxy.java | 83 ------------------- .../driver/EvitaClientReadWriteTest.java | 3 +- 6 files changed, 17 insertions(+), 134 deletions(-) delete mode 100644 evita_external_api/evita_external_api_grpc/client/src/main/java/io/evitadb/driver/StubTimeoutProxy.java diff --git a/.idea/compiler.xml b/.idea/compiler.xml index a7672d045e..ebfac65291 100644 --- a/.idea/compiler.xml +++ b/.idea/compiler.xml @@ -23,11 +23,6 @@ - - - - - @@ -37,7 +32,6 @@ - @@ -47,7 +41,6 @@ - @@ -57,7 +50,6 @@ - @@ -67,7 +59,6 @@ - @@ -77,7 +68,6 @@ - @@ -87,7 +77,6 @@ - @@ -97,7 +86,6 @@ - @@ -107,7 +95,6 @@ - @@ -117,7 +104,6 @@ - @@ -127,7 +113,6 @@ - @@ -137,27 +122,15 @@ - - - - - - - - - - - - @@ -167,7 +140,6 @@ - @@ -177,7 +149,6 @@ - @@ -187,7 +158,6 @@ - @@ -197,7 +167,6 @@ - @@ -207,7 +176,6 @@ - @@ -217,7 +185,6 @@ - @@ -227,7 +194,6 @@ - @@ -237,6 +203,7 @@ + diff --git a/evita_external_api/evita_external_api_grpc/client/src/main/java/io/evitadb/driver/EvitaClient.java b/evita_external_api/evita_external_api_grpc/client/src/main/java/io/evitadb/driver/EvitaClient.java index 7e4bcd76b7..9062915f81 100644 --- a/evita_external_api/evita_external_api_grpc/client/src/main/java/io/evitadb/driver/EvitaClient.java +++ b/evita_external_api/evita_external_api_grpc/client/src/main/java/io/evitadb/driver/EvitaClient.java @@ -125,7 +125,7 @@ public class EvitaClient implements EvitaContract { /** * Created evita service stub that returns futures. */ - private final StubTimeoutProxy evitaServiceFutureStub; + private final EvitaServiceFutureStub evitaServiceFutureStub; /** * The configuration of the evitaDB client. */ @@ -311,7 +311,7 @@ public EvitaClient( ofNullable(grpcConfigurator).ifPresent(it -> it.accept(grpcClientBuilder)); this.grpcClientBuilder = grpcClientBuilder; - this.evitaServiceFutureStub = new StubTimeoutProxy<>(grpcClientBuilder.build(EvitaServiceFutureStub.class)); + this.evitaServiceFutureStub = grpcClientBuilder.build(EvitaServiceFutureStub.class); this.reflectionLookup = new ReflectionLookup(configuration.reflectionLookupBehaviour()); this.timeout = ThreadLocal.withInitial(() -> { final LinkedList timeouts = new LinkedList<>(); @@ -789,7 +789,7 @@ private T executeWithEvitaService( ) { final Timeout timeout = this.timeout.get().peek(); try { - return lambda.apply(this.evitaServiceFutureStub.get(timeout)) + return lambda.apply(this.evitaServiceFutureStub.withDeadlineAfter(timeout.timeout(), timeout.timeoutUnit())) .get(timeout.timeout(), timeout.timeoutUnit()); } catch (ExecutionException e) { throw EvitaClient.transformException( diff --git a/evita_external_api/evita_external_api_grpc/client/src/main/java/io/evitadb/driver/EvitaClientManagement.java b/evita_external_api/evita_external_api_grpc/client/src/main/java/io/evitadb/driver/EvitaClientManagement.java index 61eec5ff1b..def31f5765 100644 --- a/evita_external_api/evita_external_api_grpc/client/src/main/java/io/evitadb/driver/EvitaClientManagement.java +++ b/evita_external_api/evita_external_api_grpc/client/src/main/java/io/evitadb/driver/EvitaClientManagement.java @@ -96,11 +96,11 @@ public class EvitaClientManagement implements EvitaManagementContract, Closeable /** * Created evita service stub. */ - private final StubTimeoutProxy evitaManagementServiceStub; + private final EvitaManagementServiceStub evitaManagementServiceStub; /** * Created evita service stub that returns futures. */ - private final StubTimeoutProxy evitaManagementServiceFutureStub; + private final EvitaManagementServiceFutureStub evitaManagementServiceFutureStub; public EvitaClientManagement(@Nonnull EvitaClient evitaClient, @Nonnull GrpcClientBuilder grpcClientBuilder) { this.evitaClient = evitaClient; @@ -109,8 +109,8 @@ public EvitaClientManagement(@Nonnull EvitaClient evitaClient, @Nonnull GrpcClie evitaClient.getConfiguration().trackedTaskLimit(), 2000 ); - this.evitaManagementServiceStub = new StubTimeoutProxy<>(grpcClientBuilder.build(EvitaManagementServiceStub.class)); - this.evitaManagementServiceFutureStub = new StubTimeoutProxy<>(grpcClientBuilder.build(EvitaManagementServiceFutureStub.class)); + this.evitaManagementServiceStub = grpcClientBuilder.build(EvitaManagementServiceStub.class); + this.evitaManagementServiceFutureStub = grpcClientBuilder.build(EvitaManagementServiceFutureStub.class); } @Nonnull @@ -513,7 +513,7 @@ private T executeWithEvitaBlockingService( final Timeout timeout = this.evitaClient.timeout.get().peek(); try { return lambda.apply( - this.evitaManagementServiceStub.get(timeout) + this.evitaManagementServiceStub.withDeadlineAfter(timeout.timeout(), timeout.timeoutUnit()) ); } catch (ExecutionException e) { throw EvitaClient.transformException( @@ -543,7 +543,7 @@ private T executeWithEvitaService( ) { final Timeout timeout = this.evitaClient.timeout.get().peek(); try { - return lambda.apply(this.evitaManagementServiceFutureStub.get(timeout)) + return lambda.apply(this.evitaManagementServiceFutureStub.withDeadlineAfter(timeout.timeout(), timeout.timeoutUnit())) .get(timeout.timeout(), timeout.timeoutUnit()); } catch (ExecutionException e) { throw EvitaClient.transformException( diff --git a/evita_external_api/evita_external_api_grpc/client/src/main/java/io/evitadb/driver/EvitaClientSession.java b/evita_external_api/evita_external_api_grpc/client/src/main/java/io/evitadb/driver/EvitaClientSession.java index bc9e580b9c..67b0af4c6f 100644 --- a/evita_external_api/evita_external_api_grpc/client/src/main/java/io/evitadb/driver/EvitaClientSession.java +++ b/evita_external_api/evita_external_api_grpc/client/src/main/java/io/evitadb/driver/EvitaClientSession.java @@ -183,11 +183,11 @@ public class EvitaClientSession implements EvitaSessionContract { /** * Entity session service that works with futures. */ - private final StubTimeoutProxy evitaSessionServiceFutureStub; + private final EvitaSessionServiceFutureStub evitaSessionServiceFutureStub; /** * Entity session service. */ - private final StubTimeoutProxy evitaSessionServiceStub; + private final EvitaSessionServiceStub evitaSessionServiceStub; /** * Contains reference to the catalog name targeted by queries / mutations from this session. */ @@ -292,8 +292,8 @@ public EvitaClientSession( this.reflectionLookup = evita.getReflectionLookup(); this.proxyFactory = schemaCache.getProxyFactory(); this.schemaCache = schemaCache; - this.evitaSessionServiceFutureStub = new StubTimeoutProxy<>(grpcClientBuilder.build(EvitaSessionServiceFutureStub.class)); - this.evitaSessionServiceStub = new StubTimeoutProxy<>(grpcClientBuilder.build(EvitaSessionServiceStub.class)); + this.evitaSessionServiceFutureStub = grpcClientBuilder.build(EvitaSessionServiceFutureStub.class); + this.evitaSessionServiceStub = grpcClientBuilder.build(EvitaSessionServiceStub.class); this.catalogName = catalogName; this.catalogState = catalogState; this.commitBehaviour = commitBehaviour; @@ -1647,7 +1647,7 @@ private T executeWithBlockingEvitaSessionService( final Timeout timeout = this.callTimeout.peek(); try { SessionIdHolder.setSessionId(getId().toString()); - return lambda.apply(this.evitaSessionServiceFutureStub.get(timeout)) + return lambda.apply(this.evitaSessionServiceFutureStub.withDeadlineAfter(timeout.timeout(), timeout.timeoutUnit())) .get(timeout.timeout(), timeout.timeoutUnit()); } catch (ExecutionException e) { throw EvitaClient.transformException( @@ -1682,7 +1682,7 @@ private T executeWithAsyncEvitaSessionService( final Timeout timeout = this.callTimeout.peek(); try { SessionIdHolder.setSessionId(getId().toString()); - return lambda.apply(this.evitaSessionServiceStub.get(timeout)); + return lambda.apply(this.evitaSessionServiceStub.withDeadlineAfter(timeout.timeout(), timeout.timeoutUnit())); } catch (ExecutionException e) { throw EvitaClient.transformException( e.getCause() == null ? e : e.getCause(), diff --git a/evita_external_api/evita_external_api_grpc/client/src/main/java/io/evitadb/driver/StubTimeoutProxy.java b/evita_external_api/evita_external_api_grpc/client/src/main/java/io/evitadb/driver/StubTimeoutProxy.java deleted file mode 100644 index 77b471d94f..0000000000 --- a/evita_external_api/evita_external_api_grpc/client/src/main/java/io/evitadb/driver/StubTimeoutProxy.java +++ /dev/null @@ -1,83 +0,0 @@ -/* - * - * _ _ ____ ____ - * _____ _(_) |_ __ _| _ \| __ ) - * / _ \ \ / / | __/ _` | | | | _ \ - * | __/\ V /| | || (_| | |_| | |_) | - * \___| \_/ |_|\__\__,_|____/|____/ - * - * Copyright (c) 2024 - * - * Licensed under the Business Source License, Version 1.1 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://github.com/FgForrest/evitaDB/blob/master/LICENSE - * - * 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. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.evitadb.driver; - - -import io.grpc.stub.AbstractStub; -import lombok.RequiredArgsConstructor; - -import javax.annotation.Nonnull; -import java.util.Objects; - -/** - * This proxy class is used to access gRPC stubs of particular type with a specific timeout. If the timeout is the same - * as the last one used, then the last stub is returned. Otherwise, a new stub with extended timeout is created. - * - * It is expected that the stubs with same timeouts will be reused, so the optimization for reusing the last used stub - * should be beneficial even if the stubs are quite cheap. - * - * @author Jan Novotný (novotny@fg.cz), FG Forrest a.s. (c) 2024 - */ -@RequiredArgsConstructor -class StubTimeoutProxy> { - /** - * The gRPC stub instance that is used to create new stubs with extended timeouts. - */ - private final T stub; - /** - * The last used gRPC stub with its associated timeout configuration. - */ - private StubTimeout lastUsedStub; - - /** - * Retrieves a gRPC stub with the specified timeout. If the timeout is different from the last used - * timeout, a new stub with the extended timeout is created. Otherwise, the previously used stub - * is returned. - * - * @param timeout The timeout configuration to be applied to the gRPC stub. - * @return A gRPC stub with the specified timeout configuration. - */ - public T get(@Nonnull Timeout timeout) { - if (this.lastUsedStub == null || !Objects.equals(this.lastUsedStub.timeout(), timeout)) { - final T stubWithExtendedTimeout = this.stub.withDeadlineAfter(timeout.timeout(), timeout.timeoutUnit()); - this.lastUsedStub = new StubTimeout<>(stubWithExtendedTimeout, timeout); - return stubWithExtendedTimeout; - } else { - return this.lastUsedStub.stub(); - } - } - - /** - * A record that holds a gRPC stub and its associated timeout configuration. - * - * @param The type of the gRPC stub. - * @param stub The gRPC stub instance. - * @param timeout The timeout configuration to be applied to the gRPC stub. - */ - private record StubTimeout( - @Nonnull T stub, - @Nonnull Timeout timeout - ) { } - -} diff --git a/evita_functional_tests/src/test/java/io/evitadb/driver/EvitaClientReadWriteTest.java b/evita_functional_tests/src/test/java/io/evitadb/driver/EvitaClientReadWriteTest.java index 2d4ea4ca6c..9fc599fbd0 100644 --- a/evita_functional_tests/src/test/java/io/evitadb/driver/EvitaClientReadWriteTest.java +++ b/evita_functional_tests/src/test/java/io/evitadb/driver/EvitaClientReadWriteTest.java @@ -1759,8 +1759,7 @@ private static EvitaManagementServiceFutureStub getManagementStubInternal( final EvitaManagementContract management = evitaClient.management(); final Field evitaManagementServiceStub = management.getClass().getDeclaredField("evitaManagementServiceFutureStub"); evitaManagementServiceStub.setAccessible(true); - return ((StubTimeoutProxy) evitaManagementServiceStub.get(management)) - .get(evitaClient.timeout.get().peek()); + return (EvitaManagementServiceFutureStub) evitaManagementServiceStub.get(management); } catch (Exception ex) { throw new RuntimeException(ex); }