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.

Sign up for GitHub

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

Jump to bottom

[Persistence] Adds atomic Update for TreeStore #861

Merged
merged 34 commits into from
Jul 31, 2023
Merged

[Persistence] Adds atomic Update for TreeStore #861

Changes from 1 commit
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
d8d8ddf
[chore] submodule pattern updates to TreeStore
dylanlott Jul 11, 2023
1a5777c
updates TreeStore to use Submodule type
dylanlott Jul 12, 2023
f87518a
[fixup] embeds TreeStoreFactory on to TreeStoreModule
dylanlott Jul 12, 2023
671598d
[fixup] TreeStoreOption and TreeStoreModule updates
dylanlott Jul 12, 2023
c988cbc
[fixup] get height provider from bus in tree tests
dylanlott Jul 17, 2023
4ae3912
[Temp] s/TreeStore/treeStore (#920)
Olshansk Jul 19, 2023
a601002
[fixup] formatting long mock definitions
dylanlott Jul 20, 2023
795ce87
[chore] submodule pattern updates to TreeStore
dylanlott Jul 11, 2023
4df81fe
[feature] adds atomic rollbacks to TreeStore
dylanlott Jul 11, 2023
f77f764
linter updates
dylanlott Jul 11, 2023
674d539
[fixup] pass testing.T to the newTestApp function
dylanlott Jul 12, 2023
5ee3f42
treestore tests use testing.T instead of err returns
dylanlott Jul 17, 2023
713017b
[fixup] privatizes treeStoreDir field
dylanlott Jul 18, 2023
7f947f9
[fixup] fix path for treestore mock generations
dylanlott Jul 19, 2023
d70a067
[fixup] use method to fetch context instead
dylanlott Jul 19, 2023
d107ed0
[fixup] adds comments and updates tests
dylanlott Jul 19, 2023
626ea5d
[fixup] adds optimize comment to save function
dylanlott Jul 19, 2023
b3ab6b0
[fixup] cleanup
dylanlott Jul 20, 2023
fc04ab0
[docs] updates package level comment
dylanlott Jul 20, 2023
2af89dd
[fixup] adds comments and updates logger use
dylanlott Jul 21, 2023
8ed31bf
dylanlott Jul 21, 2023
09097ef
[rename] s/NewSavePoint/SetSavePoint/g
dylanlott Jul 21, 2023
07b64da
[fixup] update test names and use constants in tests
dylanlott Jul 21, 2023
1ecfde6
[fixup] adds better comment to treeStore#Rollback
dylanlott Jul 21, 2023
a687768
[tests] adds a test for failing rollback
dylanlott Jul 24, 2023
800b2cb
[fixup] check rollback error value to satisfy linter
dylanlott Jul 24, 2023
0b22519
[test] adds a test for block proposal revert
dylanlott Jul 25, 2023
ba85105
[fixup] fix linter error
dylanlott Jul 25, 2023
7a7d95c
[fixup] updates rollback test to use Errors#Is
dylanlott Jul 26, 2023
0f87f0b
[fixup] prevent import cycle error errors in treestore
dylanlott Jul 27, 2023
cb4497d
[fixup] reduce export footprint of worldState
dylanlott Jul 27, 2023
6ce32dc
[docs] update rollback comment
dylanlott Jul 27, 2023
7bad92e
[fixup] s/trees_hash1/treesHash1/
dylanlott Jul 29, 2023
4d218dc
[fixup] adds a happy path test for CreateProposalBlock
dylanlott Jul 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
[fixup] use method to fetch context instead
  • Loading branch information
@dylanlott
dylanlott committed Jul 27, 2023
commit d70a067e6962e36dad4eb67b64ca0ef63c66910f
2 changes: 1 addition & 1 deletion persistence/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (p *PostgresContext) NewSavePoint() error {

// RollbackToSavepoint triggers a rollback for the current pgx transaction and the underylying submodule stores.
func (p *PostgresContext) RollbackToSavePoint() error {
ctx := context.TODO()
ctx, _ := p.getCtxAndTx()
err := p.tx.Rollback(ctx)
p.stateTrees.Rollback()
Copy link
Member

Choose a reason for hiding this comment

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

Should we pass in ctx and get back an err to append?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we pass in ctx

Yes, probably when we thread context through the module.

and get back an err to append?

It was an intentional design choice to make AtomicStore#Rollback return nothing, so I would argue no, it shouldn't. It was an attempt by me at defining errors out of existence by masking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After messing with it and testing it locally, the key might actually be to make the AtomicStore interface return an error for easier testing, but mask the error from the persistenceRWContext layer up instead, allowing it the power to handle and possibly recover from rollback errors. Thinking about this more 👍

Copy link
Member

Choose a reason for hiding this comment

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

Bump on this one.

return err
Expand Down