Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

LensRegistry does not allow registered items to be accessed within current txn #1592

Closed
Tracked by #1002
AndrewSisley opened this issue Jun 26, 2023 · 0 comments
Closed
Tracked by #1002
Assignees
Labels
area/schema Related to the schema system bug Something isn't working

Comments

@AndrewSisley
Copy link
Contributor

AndrewSisley commented Jun 26, 2023

E.g. If a transaction is explicitly passed into RegisterLens, and that txn is not commited, and then also used to query the database, then the migration registered will not be accessible to the query.

See TestSchemaMigrationQueryWithTxn and TestSchemaMigrationGetMigrationsWithTxn for tested examples.

The same issue also affects parser.SetSchema, which is also missing tests for this.

@AndrewSisley AndrewSisley added bug Something isn't working area/schema Related to the schema system labels Jun 26, 2023
@AndrewSisley AndrewSisley added this to the DefraDB v0.6 milestone Jun 26, 2023
@fredcarle fredcarle modified the milestones: DefraDB v0.6, DefraDB v0.7 Jul 4, 2023
@AndrewSisley AndrewSisley self-assigned this Jul 31, 2023
AndrewSisley added a commit that referenced this issue Aug 3, 2023
## Relevant issue(s)

Resolves #1649 #1592

## Description

Improves the way migrations handle transactions, as well as fixing a
couple of concurrency issues:

- Adds locks around the various registry properties, these maps can be
accessed concurrently and need to be protected.
- Removes the transaction continuity issue in the client.LenRegistry
interface, where db.LensRegistry() returns an object that does not
respect the transactionality of the parent store, and takes `txn`s as
input parameters to some of its functions. It does this by following the
same pattern as `db.db`. (#1649)
- Fixes the bugs in the lens package where migrations set were not
visible/accessible until after commit. They are now visible within the
transaction scope. (#1592)

It still does not provide transaction snapshot isolation, I see that
issue as relatively high effort low reward at the moment.
shahzadlone pushed a commit to shahzadlone/defradb that referenced this issue Feb 23, 2024
## Relevant issue(s)

Resolves sourcenetwork#1649 sourcenetwork#1592

## Description

Improves the way migrations handle transactions, as well as fixing a
couple of concurrency issues:

- Adds locks around the various registry properties, these maps can be
accessed concurrently and need to be protected.
- Removes the transaction continuity issue in the client.LenRegistry
interface, where db.LensRegistry() returns an object that does not
respect the transactionality of the parent store, and takes `txn`s as
input parameters to some of its functions. It does this by following the
same pattern as `db.db`. (sourcenetwork#1649)
- Fixes the bugs in the lens package where migrations set were not
visible/accessible until after commit. They are now visible within the
transaction scope. (sourcenetwork#1592)

It still does not provide transaction snapshot isolation, I see that
issue as relatively high effort low reward at the moment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/schema Related to the schema system bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants