Skip to content

Conversation

@sidhujag
Copy link

@sidhujag sidhujag commented Dec 3, 2020

Fixes syscoin/rosetta-syscoin#1

This fixes check:data being stuck on larger blocks with more changes and is synonymous with rosetta-syscoin settings here:
https://github.com/syscoin/rosetta-syscoin/blob/master/indexer/indexer.go#L130

Since the client can have those settings configurable it makes sense for CLI to have it configurable as well. We need this in order for check:data to work.

// this value is, the larger database transactions
// storage can handle (~15% of the max table size
// == max commit size). It is only applied if MemoryLimitDisabled is true.
MaxTableSize int64 `json:"max_table_size,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

let's do these as *int64...we are going to do a migration to this so that we don't infer 0 as the default

DefaultBlockBroadcastLimit = 5
DefaultStatusPort = 9090
DefaultMaxReorgDepth = 100
DefaultMaxTableSize = 256 << 20
Copy link
Contributor

Choose a reason for hiding this comment

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

Eventhough you left a comment below that explains what size this is, could you leave a note here too?

if config.MemoryLimitDisabled {
performanceOpts := database.PerformanceBadgerOptions(dataPath)
// Use an extended table size for larger commits.
performanceOpts.MaxTableSize = config.MaxTableSize
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd conditionally overwrite performanceOpts.MaxTableSize if config.MaxTableSize is not nil. (once you make the pointer change above)

@sidhujag
Copy link
Author

sidhujag commented Dec 7, 2020

@patrick-ogrady is this change relevant because you did change it upstream so it should work without this change now in coinbase/mesh-sdk-go#267

Those performance settings are good enough for it to work

@sidhujag
Copy link
Author

sidhujag commented Dec 8, 2020

Closed in favour of upstream changes that made this irrelevant

@sidhujag sidhujag closed this Dec 8, 2020
@patrick-ogrady
Copy link
Contributor

I think for now it makes sense to close this.

In the future, it may be better to add a "super performance" option that uses even more aggressive settings (so users don't need to get into the weeds with table size or value log size).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

rosetta-cli stuck on check:data

2 participants