From 67ed21365cb3bb778e488e3074f2d0aa883cb173 Mon Sep 17 00:00:00 2001 From: Mikhaylo Demianenko Date: Thu, 25 Aug 2016 15:21:12 +0200 Subject: [PATCH] Allow to start transaction with custom timeout from GraphDatabaseFacade. Rename property for timeout of idle transactions in the REST endpoint from 'dbms.transaction_timeout' to 'dbms.rest.transaction.idle_timeout'. --- .../org/neo4j/graphdb/GraphDatabaseService.java | 16 ++++++++++++++++ .../kernel/impl/factory/GraphDatabaseFacade.java | 6 ++++++ .../java/org/neo4j/test/rule/DatabaseRule.java | 6 ++++++ .../java/org/neo4j/server/AbstractNeoServer.java | 2 +- .../server/configuration/ServerSettings.java | 2 +- .../neo4j/server/TransactionTimeoutDocIT.java | 10 +++++----- .../server/modules/ExtensionInitializerTest.java | 4 ++-- .../neo4j/server/plugins/ConfigAdapterTest.java | 11 +++++------ .../shell/kernel/ReadOnlyGraphDatabaseProxy.java | 10 ++++++++-- .../neo4j/TransactionGuardIntegrationTest.java | 13 +++++++------ 10 files changed, 57 insertions(+), 23 deletions(-) diff --git a/community/graphdb-api/src/main/java/org/neo4j/graphdb/GraphDatabaseService.java b/community/graphdb-api/src/main/java/org/neo4j/graphdb/GraphDatabaseService.java index 6eef20a7a0aab..517cf22d3f410 100644 --- a/community/graphdb-api/src/main/java/org/neo4j/graphdb/GraphDatabaseService.java +++ b/community/graphdb-api/src/main/java/org/neo4j/graphdb/GraphDatabaseService.java @@ -237,6 +237,22 @@ public interface GraphDatabaseService */ Transaction beginTx(); + /** + * Starts a new {@link Transaction transaction} with custom timeout and associates it with the current thread. + * Timeout will be taken into account only when execution guard is enabled. + *

+ * All database operations must be wrapped in a transaction. + *

+ * If you attempt to access the graph outside of a transaction, those operations will throw + * {@link NotInTransactionException}. + *

+ * Please ensure that any returned {@link ResourceIterable} is closed correctly and as soon as possible + * inside your transaction to avoid potential blocking of write operations. + * + * @return a new transaction instance + */ + Transaction beginTx( long timeout ); + /** * Executes a query and returns an iterable that contains the result set. * diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacade.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacade.java index 0d8190907c7ae..aa50d5de0c825 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacade.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacade.java @@ -343,6 +343,12 @@ public Transaction beginTx() return beginTransaction( KernelTransaction.Type.explicit, AccessMode.Static.FULL ); } + @Override + public Transaction beginTx( long timeout ) + { + return beginTransaction( KernelTransaction.Type.explicit, AccessMode.Static.FULL, timeout ); + } + public InternalTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode ) { return beginTransaction( () -> spi.beginTransaction( type, accessMode ) ); diff --git a/community/kernel/src/test/java/org/neo4j/test/rule/DatabaseRule.java b/community/kernel/src/test/java/org/neo4j/test/rule/DatabaseRule.java index 96c7fc47431d8..56981b175fedf 100644 --- a/community/kernel/src/test/java/org/neo4j/test/rule/DatabaseRule.java +++ b/community/kernel/src/test/java/org/neo4j/test/rule/DatabaseRule.java @@ -147,6 +147,12 @@ public Transaction beginTx() return getGraphDatabaseAPI().beginTx(); } + @Override + public Transaction beginTx( long timeout) + { + return getGraphDatabaseAPI().beginTx( timeout ); + } + @Override public Node createNode( Label... labels ) { diff --git a/community/server/src/main/java/org/neo4j/server/AbstractNeoServer.java b/community/server/src/main/java/org/neo4j/server/AbstractNeoServer.java index 19edaa53d843a..9684f8d999a5e 100644 --- a/community/server/src/main/java/org/neo4j/server/AbstractNeoServer.java +++ b/community/server/src/main/java/org/neo4j/server/AbstractNeoServer.java @@ -265,7 +265,7 @@ private TransactionFacade createTransactionalActions() */ private long getTransactionTimeoutMillis() { - final long timeout = config.get( ServerSettings.transaction_timeout ); + final long timeout = config.get( ServerSettings.transaction_idle_timeout ); return Math.max( timeout, MINIMUM_TIMEOUT + ROUNDING_SECOND ); } diff --git a/community/server/src/main/java/org/neo4j/server/configuration/ServerSettings.java b/community/server/src/main/java/org/neo4j/server/configuration/ServerSettings.java index c9ed91b847cfa..03e928c09e992 100644 --- a/community/server/src/main/java/org/neo4j/server/configuration/ServerSettings.java +++ b/community/server/src/main/java/org/neo4j/server/configuration/ServerSettings.java @@ -232,7 +232,7 @@ private ThirdPartyJaxRsPackage createThirdPartyJaxRsPackage( String packageAndMo Setting lib_directory = pathSetting( "dbms.directories.lib", "lib" ); @Description("Timeout for idle transactions in the REST endpoint.") - Setting transaction_timeout = setting( "dbms.transaction_timeout", DURATION, "60s" ); + Setting transaction_idle_timeout = setting( "dbms.rest.transaction.idle_timeout", DURATION, "60s" ); @Internal Setting rest_api_path = setting( "unsupported.dbms.uris.rest", NORMALIZED_RELATIVE_URI, "/db/data" ); diff --git a/community/server/src/test/java/org/neo4j/server/TransactionTimeoutDocIT.java b/community/server/src/test/java/org/neo4j/server/TransactionTimeoutDocIT.java index c62dffb418c23..944eadf2a4ef2 100644 --- a/community/server/src/test/java/org/neo4j/server/TransactionTimeoutDocIT.java +++ b/community/server/src/test/java/org/neo4j/server/TransactionTimeoutDocIT.java @@ -19,12 +19,12 @@ */ package org.neo4j.server; -import java.util.List; -import java.util.Map; - import org.junit.After; import org.junit.Test; +import java.util.List; +import java.util.Map; + import org.neo4j.server.configuration.ServerSettings; import org.neo4j.test.server.ExclusiveServerTestBase; import org.neo4j.test.server.HTTP; @@ -50,7 +50,7 @@ public void stopTheServer() public void shouldHonorReallyLowSessionTimeout() throws Exception { // Given - server = server().withProperty( ServerSettings.transaction_timeout.name(), "1" ).build(); + server = server().withProperty( ServerSettings.transaction_idle_timeout.name(), "1" ).build(); server.start(); String tx = HTTP.POST( txURI(), asList( map( "statement", "CREATE (n)" ) ) ).location(); @@ -62,7 +62,7 @@ public void shouldHonorReallyLowSessionTimeout() throws Exception // Then @SuppressWarnings("unchecked") List> errors = (List>) response.get( "errors" ); - assertThat( (String) errors.get( 0 ).get( "code" ), equalTo( TransactionNotFound.code().serialize() ) ); + assertThat( errors.get( 0 ).get( "code" ), equalTo( TransactionNotFound.code().serialize() ) ); } private String txURI() diff --git a/community/server/src/test/java/org/neo4j/server/modules/ExtensionInitializerTest.java b/community/server/src/test/java/org/neo4j/server/modules/ExtensionInitializerTest.java index 275a29aa27503..7bd23bc93fdb0 100644 --- a/community/server/src/test/java/org/neo4j/server/modules/ExtensionInitializerTest.java +++ b/community/server/src/test/java/org/neo4j/server/modules/ExtensionInitializerTest.java @@ -48,7 +48,7 @@ public class ExtensionInitializerTest @Test public void testPluginInitialization() { - Config config = new Config( stringMap( ServerSettings.transaction_timeout.name(), "600" ) ); + Config config = new Config( stringMap( ServerSettings.transaction_idle_timeout.name(), "600" ) ); NeoServer neoServer = Mockito.mock( NeoServer.class, Mockito.RETURNS_DEEP_STUBS ); Mockito.when( neoServer.getConfig() ).thenReturn( config ); ExtensionInitializer extensionInitializer = new ExtensionInitializer( neoServer ); @@ -58,7 +58,7 @@ public void testPluginInitialization() assertThat( injectableProperties, Matchers.hasSize( 1 ) ); assertThat( injectableProperties, Matchers.contains( new InjectableMatcher<>( ServerSettings - .transaction_timeout.name() ) ) ); + .transaction_idle_timeout.name() ) ) ); } private class InjectableMatcher extends BaseMatcher> diff --git a/community/server/src/test/java/org/neo4j/server/plugins/ConfigAdapterTest.java b/community/server/src/test/java/org/neo4j/server/plugins/ConfigAdapterTest.java index 53872b2a1ef9d..7e99d8faf2c45 100644 --- a/community/server/src/test/java/org/neo4j/server/plugins/ConfigAdapterTest.java +++ b/community/server/src/test/java/org/neo4j/server/plugins/ConfigAdapterTest.java @@ -19,17 +19,16 @@ */ package org.neo4j.server.plugins; +import org.junit.Test; + import java.net.URI; import java.util.HashMap; -import org.junit.Test; - import org.neo4j.kernel.configuration.Config; import org.neo4j.server.configuration.ServerSettings; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; - import static org.neo4j.helpers.collection.MapUtil.stringMap; public class ConfigAdapterTest @@ -82,15 +81,15 @@ public void shouldAbleToAccessRegisteredPropertyByName() Config config = new Config( new HashMap<>(), ServerSettings.class ); ConfigAdapter wrappingConfiguration = new ConfigAdapter( config ); - assertEquals( 60000L, wrappingConfiguration.getProperty( ServerSettings.transaction_timeout.name() ) ); + assertEquals( 60000L, wrappingConfiguration.getProperty( ServerSettings.transaction_idle_timeout.name() ) ); } @Test public void shouldAbleToAccessNonRegisteredPropertyByName() { - Config config = new Config( stringMap( ServerSettings.transaction_timeout.name(), "600" ) ); + Config config = new Config( stringMap( ServerSettings.transaction_idle_timeout.name(), "600" ) ); ConfigAdapter wrappingConfiguration = new ConfigAdapter( config ); - assertEquals( "600", wrappingConfiguration.getProperty( ServerSettings.transaction_timeout.name() ) ); + assertEquals( "600", wrappingConfiguration.getProperty( ServerSettings.transaction_idle_timeout.name() ) ); } } diff --git a/community/shell/src/main/java/org/neo4j/shell/kernel/ReadOnlyGraphDatabaseProxy.java b/community/shell/src/main/java/org/neo4j/shell/kernel/ReadOnlyGraphDatabaseProxy.java index 1f745b76d74c0..c6192dc3b253c 100644 --- a/community/shell/src/main/java/org/neo4j/shell/kernel/ReadOnlyGraphDatabaseProxy.java +++ b/community/shell/src/main/java/org/neo4j/shell/kernel/ReadOnlyGraphDatabaseProxy.java @@ -59,8 +59,8 @@ import org.neo4j.kernel.api.KernelTransaction; import org.neo4j.kernel.api.security.AccessMode; import org.neo4j.kernel.impl.coreapi.InternalTransaction; -import org.neo4j.kernel.internal.GraphDatabaseAPI; import org.neo4j.kernel.impl.store.StoreId; +import org.neo4j.kernel.internal.GraphDatabaseAPI; public class ReadOnlyGraphDatabaseProxy implements GraphDatabaseService, GraphDatabaseAPI, IndexManager { @@ -103,10 +103,16 @@ public Transaction beginTx() return actual.beginTx(); } + @Override + public Transaction beginTx( long timeout ) + { + return actual.beginTx( timeout ); + } + @Override public Result execute( String query ) { - return execute( query, Collections.emptyMap() ); + return execute( query, Collections.emptyMap() ); } @Override diff --git a/integrationtests/src/test/java/org/neo4j/TransactionGuardIntegrationTest.java b/integrationtests/src/test/java/org/neo4j/TransactionGuardIntegrationTest.java index e625043d443c1..82fac1752551a 100644 --- a/integrationtests/src/test/java/org/neo4j/TransactionGuardIntegrationTest.java +++ b/integrationtests/src/test/java/org/neo4j/TransactionGuardIntegrationTest.java @@ -45,8 +45,9 @@ import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.configuration.Settings; import org.neo4j.kernel.guard.GuardTimeoutException; -import org.neo4j.kernel.impl.factory.CommunityFacadeFactory; +import org.neo4j.kernel.impl.factory.CommunityEditionModule; import org.neo4j.kernel.impl.factory.DataSourceModule; +import org.neo4j.kernel.impl.factory.DatabaseInfo; import org.neo4j.kernel.impl.factory.EditionModule; import org.neo4j.kernel.impl.factory.GraphDatabaseFacade; import org.neo4j.kernel.impl.factory.GraphDatabaseFacadeFactory; @@ -63,9 +64,9 @@ import org.neo4j.shell.impl.CollectingOutput; import org.neo4j.shell.impl.SameJvmClient; import org.neo4j.shell.kernel.GraphDatabaseShellServer; -import org.neo4j.test.CleanupRule; import org.neo4j.test.TestGraphDatabaseFactory; import org.neo4j.test.TestGraphDatabaseFactoryState; +import org.neo4j.test.rule.CleanupRule; import org.neo4j.test.server.HTTP; import org.neo4j.time.FakeClock; @@ -227,7 +228,6 @@ private Map,String> getSettingsMap() GraphDatabaseSettings.auth_enabled, "false" ); } - private String transactionUri(CommunityNeoServer neoServer) { return neoServer.baseUri().toString() + "db/data/transaction"; @@ -306,9 +306,9 @@ private class GuardTestServer extends CommunityNeoServer private class GuardTestGraphDatabaseFactory extends TestGraphDatabaseFactory { - private CommunityFacadeFactory customFacadeFactory; + private GraphDatabaseFacadeFactory customFacadeFactory; - GuardTestGraphDatabaseFactory( CommunityFacadeFactory customFacadeFactory ) + GuardTestGraphDatabaseFactory( GraphDatabaseFacadeFactory customFacadeFactory ) { this.customFacadeFactory = customFacadeFactory; } @@ -322,13 +322,14 @@ protected GraphDatabaseBuilder.DatabaseCreator createImpermanentDatabaseCreator( } } - private class GuardCommunityFacadeFactory extends CommunityFacadeFactory + private class GuardCommunityFacadeFactory extends GraphDatabaseFacadeFactory { private Clock clock; GuardCommunityFacadeFactory( Clock clock ) { + super( DatabaseInfo.COMMUNITY, CommunityEditionModule::new); this.clock = clock; }