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

Shouldn't CreateBucketIfNotExists() try to open the bucket first? #118

Closed
xeoncross opened this issue Sep 8, 2018 · 3 comments · Fixed by #532
Closed

Shouldn't CreateBucketIfNotExists() try to open the bucket first? #118

xeoncross opened this issue Sep 8, 2018 · 3 comments · Fixed by #532
Assignees

Comments

@xeoncross
Copy link

CreateBucketIfNotExists() currently tries to create the bucket as the first operation - instead of trying to find the bucket. For use cases where it's highly likely the bucket doesn't exist, it makes sense to create first, and on fail read.

However, having the reverse would be a really handy way to open buckets in cases where a lot of buckets are automatically generated and kept around (the system has repeated reads).

I plan on opening many longer-living buckets during runtime (I can't pre-caculated them to allocate them on startup). Since I will be reading these buckets a couple orders of magnitude more than I create them - I would rather not incur the double-seek penalty for trying to create, then read, every time I use the bucket.

The solution either seems to be:

  • To cache the bucket's existence: (if created { tx.Bucket(name) } else { tx.CreateBucketIfNotExists(name))
  • Or to try to read, and on fail create:
b := tx.Bucket(name)
if b == nil {
b, err := tx.CreateBucket(name)
}

As a side note unrelated to this: Either of these are prone to race conditions in a way that pre-allocating them on startup would solve, if only I could.

@zwass
Copy link

zwass commented Sep 26, 2018

Is there a race condition problem here even if you run this within an Update transaction?

@xeoncross
Copy link
Author

@zwass I wasn't clear enough so I'm not exactly sure what I meant. However, I don't think there is an actual race-condition; as in an undefined data state. I think I was talking about the order that View or Update transactions could be issued.

@missinglink
Copy link
Contributor

The subbucket cache lookup at the top of a call to Bucket() is presumably very cheap.

I don't understand the locking well enough, but assuming there is no lock on b.buckets then this might be faster:

func (b *Bucket) CreateBucketIfNotExists(key []byte) (*Bucket, error) {
	
	// check subbucket cache for existing bucket
	if b.buckets != nil {
		if child := b.buckets[string(key)]; child != nil {
			return child
		}
	}

	// create new bucket
	child, err := b.CreateBucket(key)
	if err != nil {
		return nil, err
	}

	return child, nil
}

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