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

cmd, core, ethdb: enable Pebble on 32 bits and OpenBSD too #28335

Merged
merged 2 commits into from
Oct 13, 2023

Conversation

karalabe
Copy link
Member

Supposedly upstream supports both now.

@karalabe karalabe added this to the 1.13.4 milestone Oct 13, 2023
@holiman
Copy link
Contributor

holiman commented Oct 13, 2023

ethdb\pebble\pebble.go:150:21: cannot use 4 << 30 - 1 (untyped int constant 4294967295) as int value in assignment (overflows)

@holiman
Copy link
Contributor

holiman commented Oct 13, 2023

It's a constant we took from pebble, and have updated it upstream now: https://github.com/cockroachdb/pebble/blob/master/open.go#L57C44-L57C44

	// The max memtable size is limited by the uint32 offsets stored in
	// internal/arenaskl.node, DeferredBatchOp, and flushableBatchEntry.
	//
	// We limit the size to MaxUint32 (just short of 4GB) so that the exclusive
	// end of an allocation fits in uint32.
	//
	// On 32-bit systems, slices are naturally limited to MaxInt (just short of
	// 2GB).
	maxMemTableSize = constants.MaxUint32OrInt
const (
	// oneIf64Bit is 1 on 64-bit platforms and 0 on 32-bit platforms.
	oneIf64Bit = ^uint(0) >> 63

	// MaxUint32OrInt returns min(MaxUint32, MaxInt), i.e
	// - MaxUint32 on 64-bit platforms;
	// - MaxInt on 32-bit platforms.
	// It is used when slices are limited to Uint32 on 64-bit platforms (the
	// length limit for slices is naturally MaxInt on 32-bit platforms).
	MaxUint32OrInt = (1<<31)<<oneIf64Bit - 1
)

Copy link
Member

@gballet gballet left a comment

Choose a reason for hiding this comment

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

Tried with --dev and --synctarget on 64bit OpenBSD, geth compiles, starts and starts syncing

@karalabe
Copy link
Member Author

@holiman Thanks. Updated the constant to be in line with Pebble. PTAL

@karalabe
Copy link
Member Author

@gballet TY!

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM

@karalabe karalabe merged commit 509a64f into ethereum:master Oct 13, 2023
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
…28335)

* cmd, core, ethdb: enable Pebble on 32 bits and OpenBSD too

* ethdb/pebble: use Pebble's internal constant calculation
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
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.

3 participants