gchq / gchq/sleeper

Tidy methods to commit to state store

Open
#6,520 0 comments 0 reactions 0 assignees View on GitHub
statestore-module
Dominant language
Java
Stars
107
Forks
29
Avg merge
19h 46m
Merged PRs (30d)
141

Description

### User Story

As a user of Sleeper who wants to add a transaction directly to the state store, I want the code I find for this to do the right thing by default, so that the transaction will be handled consistently with those added internally by Sleeper.

As a developer, I want the methods to commit to the state store to be in one place and clear about the distinction between them, so that I know whether an update to the state store will be done synchronously, asynchronously, or by a method set in configuration.

### Description / Background

Currently to update the state store, you can either do it synchronously or asynchronously. The methods for this are in two different places, and neither of them are methods on the state store object.

The existing methods on the state store to add a transaction are currently only used by the state store committer. A user would likely expect this to make an asynchronous commit to the state store, but in fact they should never call that method.

We'd like to tidy this so that when a user wants to commit directly to the state store, they will do the right thing by default, with asynchronous commit.

### Technical Notes / Implementation Details

#### Current state

There's StateStore.addFilesTransaction and StateStore.addPartitionTransaction which are currently only used directly by the state store committer.

There's the synchronousCommit method on all the transaction objects, e.g. AddFilesTransaction.

There's SqsFifoStateStoreCommitRequestSender, which sends asynchronous commits.

Everywhere in the system that supports asynchronous commits has its own logic for whether to call the commit request sender or the synchronousCommit method.

#### Changes

It seems like we need to move the existing options onto the state store interface. There are a couple of options for how we structure this:

- Use the existing methods, and add options to AddTransactionRequest for synchronous/asynchronous/committer
- Make separate methods for synchronous/asynchronous/committer

In either case we'll want to make sure that asynchronous commit is the one most people choose by default. We can explain why you won't want to use synchronous commit most of the time, as it has an impact on throughput of the state store.

We'll need to adjust the logic that's currently on the transactions' synchronousCommit methods, so that it can be called from the state store object if it's necessary.

#### Creation of a state store object

Since some extra parts will be needed inside the state store, we can adjust the builder and static constructors for DynamoDBTransactionLogStateStore to be specific about what you want to be able to do with it.

For asynchronous commits we'll need a commit request sender.

The resulting state store should fail if the user attempts a type of commit that it's not deliberately configured to be able to do. We can only accept synchronous commits if they are specifically permitted when creating the state store. We could specifically permit just certain types of transaction for synchronous commit.

We can have static methods to create state stores for the most common use cases where the user wants read-only access, or when they want to be able to commit asynchronously. The default should be that the user gets a state store that will refuse to make synchronous commits.

Contributor guide

Open the contributing guide

Research direction

Start by tracing StateStore.addFilesTransaction and StateStore.addPartitionTransaction, the transaction synchronousCommit methods, and SqsFifoStateStoreCommitRequestSender. Then inspect DynamoDBTransactionLogStateStore builders and static constructors. Done means commit methods are organized on the state store, asynchronous commit is the default, and unsupported synchronous commits fail unless explicitly configured.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
database
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.