feat(api): Idempotent Resource Creation #298
Conversation
|
|
Hi @justenwalker, thank you for your contribution! We like what this change allows and discussed internally - our one question is: was there a reason a new column was created instead of using the already existing id column? That serves as our PK in our database currently and it makes sense to add that to the data models and use it as our idempotency key for this feature. |
If you check the discussion; that was an option that I presented; however my concerns are:
|
| final String indexName = String.format("%s_actor_type_idempotency_key", table); | ||
| final Field<UUID> idempotencyKey = DSL.field(columnName, SQLDataType.UUID.nullable(true)); | ||
| ctx.alterTable(table).addColumnIfNotExists(idempotencyKey).execute(); | ||
| ctx.createUniqueIndex(indexName).on(table, columnName, "actor_type").execute(); |
There was a problem hiding this comment.
Perhaps actor_type should come first in the index instead of the idempotency_key; as the actor_type is sort of like a namespace for the idempotency_key.
| } catch (ConfigNotFoundException e) { | ||
| // not found, so must create | ||
| } |
There was a problem hiding this comment.
I'm not too happy with this. I suspect this is not a situation that you can get in, since the existence of the sourcedef implies it has at least one version (i would think). But i have to catch this due to buildSourceDefinitionRead throwing it.
| } catch (ConfigNotFoundException e) { | ||
| // not found, so must create | ||
| } |
There was a problem hiding this comment.
I'm not too happy with this. I suspect this is not a situation that you can get in, since the existence of the destdef implies it has at least one version (i would think). But i have to catch this due to buildDestinationDefinitionRead throwing it.
What
Based on [API] Idempotent Resource Creation Discussion.
This change adds an
idempotencyKeyto the API and database tables. This enables an idempotent way to create resources; preventing duplicate creation of the same resource for retries and concurrent create requests.Why
And Idempotency key of some kind is necessary in order to prevent duplicates from being created in concurrent or retried requests.
For example, I often run into this scenario; especially if the Airbyte Server is under load (when it starts returning 504/502 errors):
The more retries, the more duplicates.
Since no resource has any unique constraint besides its UUID (which is generated on creation) -- the server will gladly accept multiple create requests for an identical resource.
How
idempotency_keyto the database tables oforganizations,users,workspaces,connections,actors, andactor_definitions.idempotency_keyrows and fast lookup byidempotency_key.idempotencyKeyvalue on create for these resources.Recommended reading order
Database Migrations
API Changes
API Server Mappings
Server Handlers
Config Model Updates
Persistence Changes
Can this PR be safely reverted / rolled back?
Contains API changes and DB migrations.
🚨 User Impact 🚨
This shouldn't be a breaking change, as the new idempotencyKey is an optional parameter; and behavior should
not be different than it currently is if it is not given in the API Calls.