[Dynamic Protocol State] Protocol state storage #4559

durkmurder · 2023-07-13T12:34:11Z

Context

This PR implements a storage system for protocol state along with auxiliary data structures, storage access layer and caching strategy. Some of important concepts:

flow.ProtocolStateEntry is stored in DB and is designed in a way to minimize storage overhead.
flow.RichProtocolStateEntry decorates basic flow.ProtocolStateEntry with additional data that has to be queried from storage layer (such as EpochSetup events).
storage.ProtocolState operates with flow.ProtocolStateEntry for storing and flow.RichProtocolStateEntry for querying
badger.ProtocolState uses a cache with no-op storing capability, in this case cache is populated only on queries. Cache itself operates with flow.RichProtocolStateEntry to avoid querying extra data.

…c parts

…ests. Part 1

…d fixed usages. Updated signer indices

…nding on context. Updated usages. Fixed tests

…/6232-static-identity-model

…nctions

…value

…ing on DB lookups.

…ring identities

Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>

…-protocol-state-storage-layer

engine/access/rpc/engine.go

model/flow/protocol_state.go

AlexHentschel · 2023-08-25T00:52:14Z

model/flow/protocol_state_test.go

+	// Common situation during the epoch setup phase for epoch N+1
+	//  * we are currently in Epoch N
+	//  * previous epoch N-1 is known (specifically EpochSetup and EpochCommit events)
+	//  * network is currently in the setup phase for the next epoch, i.e. EpochSetup event (starting setup phase) has already been observed
+	t.Run("setup-phase", func(t *testing.T) {


For the staking phase we test both scenarios:

previous epoch unknown: first epoch after spork

previous epoch unknown: common case for later epochs in spork

However, for the setup phase we don't test the scenario where no prior epoch is known. I think we should extend the test here.

same for testing the epoch commit phase

added respective TODO to issue #4649

AlexHentschel · 2023-08-25T01:46:10Z

storage/badger/protocol_state.go

+// ByBlockID returns the protocol state by block ID.
+func (s *ProtocolState) ByBlockID(blockID flow.Identifier) (*flow.RichProtocolStateEntry, error) {
+	tx := s.db.NewTransaction(false)
+	defer tx.Discard()
+	return s.byBlockID(blockID)(tx)
+}


performance concern

I am would suggest to also include a cache for the secondary index here as well. The Headers implementation provides a nice example, where we have a secondary index by height that is also backed by a cache

added as TODO to issue #4649

AlexHentschel · 2023-08-25T02:05:22Z

storage/badger/protocol_state.go

+)
+
+// ProtocolState implements persistent storage for storing entities of protocol state.
+// Protocol state uses an embedded cache without storing capabilities(store happens on first retrieval) to avoid unnecessary


I am wondering how much of a challenge it would be to actually cache the RichProtocolStateEntry on store. At the moment, we insert a ProtocolStateEntry (non-rich) -- presumably this is the reason for not storing the RichProtocolStateEntry. However, to me this seems to be an artifact that could indicate room for improvement in the software design:

I think for about 95% of the blocks that we store, we need the RichProtocolStateEntry state later. This is because we finalize 95% of all proposed blocks, i.e. the blocks have children; I would guess we that we query the RichProtocolStateEntry when processing a block's child.

To me it would be most natural if the process evolving the identity table produced the RichProtocolStateEntry. The RichProtocolStateEntry would ideally provide methods to generate/return its condensed version.

added respective TODO to issue #4649

AlexHentschel

tried to address as many comments as I could and added most of the remaining TODOs to #4649

model/flow/protocol_state.go

storage/protocol_state.go

AlexHentschel · 2023-08-25T02:58:12Z

model/flow/protocol_state_test.go

+	// Common situation during the epoch setup phase for epoch N+1
+	//  * we are currently in Epoch N
+	//  * previous epoch N-1 is known (specifically EpochSetup and EpochCommit events)
+	//  * network is currently in the setup phase for the next epoch, i.e. EpochSetup event (starting setup phase) has already been observed
+	t.Run("setup-phase", func(t *testing.T) {


added respective TODO to issue #4649

AlexHentschel · 2023-08-25T02:58:19Z

storage/badger/protocol_state.go

+)
+
+// ProtocolState implements persistent storage for storing entities of protocol state.
+// Protocol state uses an embedded cache without storing capabilities(store happens on first retrieval) to avoid unnecessary


added respective TODO to issue #4649

Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>

jordanschalm · 2023-09-07T15:31:20Z

Failing test:

=== RUN   TestNewRichProtocolStateEntry
=== RUN   TestNewRichProtocolStateEntry/staking-root-protocol-state
=== RUN   TestNewRichProtocolStateEntry/staking-phase
=== RUN   TestNewRichProtocolStateEntry/setup-phase
    protocol_state_test.go:84: 
        	Error Trace:	/home/runner/work/flow-go/flow-go/model/flow/protocol_state_test.go:84
        	Error:      	Received unexpected error:
        	            	inconsistent EpochCommit for constucting RichProtocolStateEntry, next protocol state states ID 0000000000000000000000000000000000000000000000000000000000000000 while input event has ID f18f47848fb293468f641c33863dca9e5278fa8e9690f77f7dc96e954ef9221b
        	Test:       	TestNewRichProtocolStateEntry/setup-phase
--- FAIL: TestNewRichProtocolStateEntry (0.03s)
    --- PASS: TestNewRichProtocolStateEntry/staking-root-protocol-state (0.01s)
    --- PASS: TestNewRichProtocolStateEntry/staking-phase (0.01s)
    --- FAIL: TestNewRichProtocolStateEntry/setup-phase (0.02s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0xef6a5d]

goroutine 167 [running]:
testing.tRunner.func1.2({0x109aa20, 0x1be4fb0})
	/opt/hostedtoolcache/go/1.20.7/x64/src/testing/testing.go:1526 +0x24e
testing.tRunner.func1()
	/opt/hostedtoolcache/go/1.20.7/x64/src/testing/testing.go:1529 +0x39f
panic({0x109aa20, 0x1be4fb0})
	/opt/hostedtoolcache/go/1.20.7/x64/src/runtime/panic.go:884 +0x213
github.com/onflow/flow-go/model/flow_test.TestNewRichProtocolStateEntry.func3(0xc000201380?)
	/home/runner/work/flow-go/flow-go/model/flow/protocol_state_test.go:86 +0xfd
testing.tRunner(0xc0004389c0, 0x131c0f0)
	/opt/hostedtoolcache/go/1.20.7/x64/src/testing/testing.go:1576 +0x10b
created by testing.(*T).Run
	/opt/hostedtoolcache/go/1.20.7/x64/src/testing/testing.go:1629 +0x3ea
FAIL	github.com/onflow/flow-go/model/flow	0.380s

…nflow/flow-go into yurii/5529-dynamic-protocol-state-storage-layer

durkmurder added 25 commits July 3, 2023 20:02

Changed core identity type to be constructured from static and dynami…

1889c47

…c parts

Fixed usages of Identity

81ca0c8

Updated usages of flow.Identity to use flow.IdentitySkeleton. Fixed t…

c19603d

…ests. Part 1

Fixed compilation issues in tests

0f333f3

Linted

cff04e2

Fixed more tests

b301877

Updated Replicas to return identity skeleton. Updated implementors an…

a0599fb

…d fixed usages. Updated signer indices

Fixed compilation issues for cluster committee

8c6bfc2

Updated mocks

1cbbe9b

Updated hotstuff committee to return skeleton and full identites depe…

19d5ada

…nding on context. Updated usages. Fixed tests

Fixed more tests

6682f27

Merge branch 'master' of https://github.com/onflow/flow-go into yurii…

7bfd22f

…/6232-static-identity-model

LInted

47e9de5

Added protocol state entry data structure and added much needed ID fu…

d436320

…nctions

Added interface for ProtocolState storage

81f77ce

Added protocol state database operations

5299054

Added protocol state storage implementation

d235fe9

Updated implementation of protocol state to correctly return indexed …

606a729

…value

Added RichProtocolStateEntry for storing all needed data without rely…

0114b3c

…ing on DB lookups.

Updated protocol state DB to enrich protocol state entry with extra data

cb20acc

Added godoc for badger.ProtocolState

b1d0702

More godoc updates

3840c65

Updated fixtures, added basic test for storage. Fixed issue with orde…

21af768

…ring identities

Updated tests to fully cover building of RichProtocolStateEntry

ba61c28

Added extra test for badger operations. Linted

b66d0e1

durkmurder requested review from zhangchiqing, jordanschalm and AlexHentschel July 13, 2023 12:34

durkmurder assigned jordanschalm and AlexHentschel Jul 13, 2023

Apply suggestions from code review

7556712

Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>

Base automatically changed from yurii/6232-static-identity-model to feature/dynamic-protocol-state August 23, 2023 17:58

jordanschalm requested review from yhassanzadeh13 and peterargue as code owners August 23, 2023 17:58

Alexander Hentschel and others added 8 commits August 23, 2023 19:57

minor goDoc updates

1ef613a

removed unnecessary whitespaces

60be9c4

Merge branch 'feature/dynamic-protocol-state' into yurii/5529-dynamic…

1227374

…-protocol-state-storage-layer

refined goDoc

be31002

refined goDoc part 2

9a80526

extended goDoc for tests

e69609a

added TODOs reflecting review comments

2fea77a

fixed merge artifacts.

7a16b08

AlexHentschel reviewed Aug 25, 2023

View reviewed changes

AlexHentschel mentioned this pull request Aug 25, 2023

[Dynamic Protocol State] Remaining work and ToDos #4649

Closed

19 tasks

Alexander Hentschel added 3 commits August 24, 2023 19:31

refined and extended goDoc

279a464

more goDoc extensions

3f89749

extended badger.ProtocolState interface documentation

bd54989

AlexHentschel approved these changes Aug 25, 2023

View reviewed changes

AlexHentschel removed request for zhangchiqing, yhassanzadeh13 and peterargue August 25, 2023 03:00

jordanschalm and others added 2 commits August 25, 2023 14:19

remove unused fn WaitForServerStart

170109a

Update model/flow/protocol_state.go

f91d5c8

Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>

jordanschalm approved these changes Sep 7, 2023

View reviewed changes

durkmurder added 2 commits September 7, 2023 23:14

Fixed sanity check

ddaf7ed

Merge branch 'feature/dynamic-protocol-state' of https://github.com/o…

628d61f

…nflow/flow-go into yurii/5529-dynamic-protocol-state-storage-layer

durkmurder merged commit 620d901 into feature/dynamic-protocol-state Sep 7, 2023

durkmurder deleted the yurii/5529-dynamic-protocol-state-storage-layer branch September 7, 2023 21:48

durkmurder mentioned this pull request Nov 29, 2023

[Dynamic Protocol State] TODOs and refactoring, part 2 #5080

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Dynamic Protocol State] Protocol state storage #4559

[Dynamic Protocol State] Protocol state storage #4559

durkmurder commented Jul 13, 2023

AlexHentschel Aug 25, 2023

AlexHentschel Aug 25, 2023

AlexHentschel Aug 25, 2023 •

edited

Loading

AlexHentschel Aug 25, 2023

AlexHentschel Aug 25, 2023

AlexHentschel Aug 25, 2023

AlexHentschel left a comment

AlexHentschel Aug 25, 2023

AlexHentschel Aug 25, 2023

jordanschalm commented Sep 7, 2023

[Dynamic Protocol State] Protocol state storage #4559

[Dynamic Protocol State] Protocol state storage #4559

Conversation

durkmurder commented Jul 13, 2023

Context

AlexHentschel Aug 25, 2023

Choose a reason for hiding this comment

AlexHentschel Aug 25, 2023

Choose a reason for hiding this comment

AlexHentschel Aug 25, 2023 • edited Loading

Choose a reason for hiding this comment

performance concern

AlexHentschel Aug 25, 2023

Choose a reason for hiding this comment

AlexHentschel Aug 25, 2023

Choose a reason for hiding this comment

AlexHentschel Aug 25, 2023

Choose a reason for hiding this comment

AlexHentschel left a comment

Choose a reason for hiding this comment

AlexHentschel Aug 25, 2023

Choose a reason for hiding this comment

AlexHentschel Aug 25, 2023

Choose a reason for hiding this comment

jordanschalm commented Sep 7, 2023

AlexHentschel Aug 25, 2023 •

edited

Loading