Skip to content
New issue

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

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

Already on GitHub? # to your account

Expose transaction timeout and metadata in the API #518

Merged
merged 6 commits into from
Aug 2, 2018

Conversation

lutovich
Copy link
Contributor

@lutovich lutovich commented Jul 26, 2018

This is the first draft of the API to allow transaction timeout and/or metadata be set for explicit and auto-commit transactions. Both settings can be specified in a TransactionConfig object which can be created like this:

TransactionConfig config = TransactionConfig.builder()
            .withTimeout(Duration.ofSeconds(5))
            .withMetadata(Collections.singletonMap("key", "value"))
            .build();

Explicit transactions can take config like this:

Transaction tx = session.beginTransaction(config);

and auto-commit transactions accept it in various overloads of Session#run() and Session#runAsync():

session.run("CREATE ()", config);

session.runAsync("RETURN $x", Collections.singletonMap("x", 1) config);

Transaction functions accept config like this:

session.readTransaction(tx -> {...}, config);

session.writeTransactionAsync(tx -> {...}, config);

@lutovich lutovich force-pushed the 1.7-tx-meta-and-timeout branch 2 times, most recently from dbc0e4c to 910aff1 Compare July 27, 2018 12:30
@zhenlineo zhenlineo force-pushed the 1.7-tx-meta-and-timeout branch from ed68e6c to 7e0c16a Compare July 27, 2018 14:18
@lutovich lutovich force-pushed the 1.7-tx-meta-and-timeout branch from 7e0c16a to 86d9d36 Compare July 30, 2018 12:27
This is the first draft of the API to allow transaction timeout and/or
metadata be set for explicit and auto-commit transactions. Both
settings can be specified in a `TransactionConfig` object which can be
created like this:

```
TransactionConfig config = TransactionConfig.builder()
            .withTimeout(Duration.ofSeconds(5))
            .withMetadata(Collections.singletonMap("key", "value"))
            .build();
```

Explicit transactions can take config like this:

```
Transaction tx = session.beginTransaction(config);
```

and auto-commit transactions accept it in various overloads of
`Session#run()` and `Session#runAsync()`:

```
session.run("CREATE ()", config);

session.runAsync("RETURN $x", Collections.singletonMap("x", 1) config);
```

Transaction functions accept config like this:

```
session.readTransaction(tx -> {...}, config);

session.writeTransactionAsync(tx -> {...}, config);
```

There also exists a possibility to specify default transaction
configuration per driver. It can be configured in the driver's config:

```
Config driverConfig = Config.build()
            .withDefaultTransactionConfig(txConfig)
            .toConfig();
```

More tests and javadocs will be added in subsequent commits.
It might be a nice thing to have but we are not yet sure about it.
Should be easy to add later, if required.
@lutovich lutovich force-pushed the 1.7-tx-meta-and-timeout branch from c6282a1 to b355c4b Compare August 1, 2018 14:14
Copy link
Contributor

@zhenlineo zhenlineo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good. Nice java doc

{
if ( config != null && !config.isEmpty() )
{
return txConfigNotSupported();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is amazing! Finally, we do not need to check with server version anymore!

*/
public class TransactionConfig
{
private static final TransactionConfig EMPTY = builder().build();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we now have this builder style, shall we also give Driver config a builder style?
So we deprecate the old method and then provides a new builder API. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to just change it in the next major release, tbh

otherTx.run( "MATCH (n:Node) SET n.prop = 1" ).consume();

TransactionConfig config = TransactionConfig.builder()
.withTimeout( ofSeconds( 1 ) )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it need 1s? Can we give it 1ms to speed up this test? This 1s might slow down my fancy new compute to run tests. (The minimal legal value server accept is 1ms)

Similar for the async one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed to keep your computer cool

@zhenlineo zhenlineo merged commit 45a1a15 into neo4j:1.7 Aug 2, 2018
@lutovich lutovich deleted the 1.7-tx-meta-and-timeout branch August 2, 2018 08:24
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants