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

soroban-rpc: Add in-memory event store backed by circular buffer #361

Merged
merged 6 commits into from
Jan 27, 2023

Conversation

tamirms
Copy link
Contributor

@tamirms tamirms commented Jan 27, 2023

What

Add in-memory event store backed by circular buffer. This code is taken from #355 . To minimize merge conflicts, the ingestion and DB code changes will be included in a separate PR.

Why

This is necessary to complete stellar/go#4718

Known limitations

[N/A]

@tamirms tamirms requested a review from a team January 27, 2023 09:38
@2opremio
Copy link
Contributor

2opremio commented Jan 27, 2023

As I mentioned before, I would consider some modifications to the data structure so that we can (mostly) allocate it in advance

Also, it would be good to know how much memory could be consumed by this (e.g. assuming full ledgers and certain amount of events per operation)

@tamirms
Copy link
Contributor Author

tamirms commented Jan 27, 2023

I started looking into @2opremio suggestion to take advantage of the fact that there are at most 1000 operations per ledger. I used the following representation:

type bucket struct {
	ledgerSeq uint32
	ops  []opEvents
}

type opEvents struct {
	events  []xdr.ContractEvent
	txIndex uint32
	opIndex uint32
}

// MemoryStore is an in-memory store of soroban events.
type MemoryStore struct {
	lock sync.RWMutex
	// buckets is a circular buffer where each cell represents
	// all events occurring within a specific ledger.
	buckets []bucket
	// length is equal to the number of ledgers contained within
	// the circular buffer.
	length uint32
	// start is the index of the head in the circular buffer.
	start uint32
}

but the code started to be more complex and less readable because of the fact that only operations per ledger are fixed (not transactions). I'm going to file an issue to investigate improving this representation in the future

Copy link
Contributor

@tsachiherman tsachiherman left a comment

Choose a reason for hiding this comment

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

Few small comments, but looks great otherwise.

cmd/soroban-rpc/internal/events/events.go Outdated Show resolved Hide resolved
cmd/soroban-rpc/internal/events/events.go Outdated Show resolved Hide resolved
cmd/soroban-rpc/internal/events/events.go Outdated Show resolved Hide resolved
cmd/soroban-rpc/internal/events/events.go Outdated Show resolved Hide resolved
@tamirms
Copy link
Contributor Author

tamirms commented Jan 27, 2023

@tsachiherman can you take another look? I just pushed a commit which addresses your latest round of feedback

Copy link
Contributor

@tsachiherman tsachiherman left a comment

Choose a reason for hiding this comment

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

looks great! thanks for the changes.

@tamirms tamirms merged commit 0e49115 into main Jan 27, 2023
@tamirms tamirms deleted the events-in-memory branch January 27, 2023 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants