storm
storm copied to clipboard
asdine
Add support for ULID
See https://github.com/oklog/ulid
This ID has some interesting characteristics that would be a good fit for Storm, me thinks, one of them being the "lexicographically sortable".
It should live in a separate package, I guess, but I think it would be useful as a built-in option similar to AutoIncrement -- `AutoULID``?
If there this sound interesting I can take a stab on implementing it.
Thinking about it a little, one of the plus sides of these IDs is that they can be set before persisted to DB, so if this is added we should add a way to just get a new ID -- still valuable as me and others then do not have to set up pooling for rand etc.
I think there should be a way to setup the incrementer that would be called by Storm everytime it needs a new ID.
I think there should be a way to setup the incrementer that would be called by Storm everytime it needs a new ID.
Yes, we should add some kind of IDProvider interface, which defaults to the current
So we could do something ala:
db, cleanup := createDB(b, AutoIncrement(ULID()))
Or something.
And then add a GetIDProvider() func, which in some cases would fail if not inside a transaction.
The AutoIncrement setting name is unfortunate, maybe IDProvider... I see the GoDoc is still talking about BoltDB's NextSequence, but it seems to have been replaced with some custom; what is the reason behind that @asdine?
If i remember well, it was because NextSequence doesn't allow us to set the initial value of the counter.
Also, the AutoIncrement option is deprecated and should be removed soon.
Concerning the IDProvider it seems to be a good idea.
Currently, there is a metadata bucket stored inside each bucket that contains the codec used and the current counter.
It could be used to call the IDProvider to generate the next id by passing the last one used.
db, err := storm.Open("my.db", IDProvider(ULID()))
type IDProvider interface {
NextID(last interface{}) interface{}
}
I'm not a big fan of empty interfaces but i don't think we have a choice here wdyt @bep ?
Would it not be be better if it was non race condition exposed ? In other words ask the metaBucket / IDProvider for the next ID, because only it knows.
Or
After a commit just always send back the ID in the return call. Then there is no chance of race condition.
@asdine IDProvider can't be both this interface, and a function that returns func(*DB) error, per:
func Open(path string, stormOptions ...func(*DB) error) (*DB, error)
I envisioned something like:
type IDProviderFunc func(last interface{}) interface{}
func IDProvider(IDProviderFunc) func(*DB) error {...}
db, err := storm.Open("my.db", IDProvider(storm.ULID))
I'm not a huge fan of the empty interfaces here either, but alternatives do not come to mind immediately.
@joeblew99 this is why @bep suggests that some cases would error outside a transaction, otherwise, a mutex per bucket is probably required, but all this would be transparent to the user/provider-implementer.
@joeblew99 Actually, the meta.Increment call is done within a write transaction so it's not subject to race conditions. Also the call to the IDProviderFunc will be done exclusively inside the write transaction.
@pdf Yes sorry, i picked the wrong name and my example is confusing. Your solution is cool, a function should be enough.
type IDProviderFunc func(last interface{}) interface{}
What is the purpose of last? I thought this was either very stateful (meta.Increment) or very stateless (ULID, UUID...).
This supports both stateful and stateless - last would be the previous int for auto-increment, and ignored by the ULID implementation.
Right, so declaring the IDProvider attaches it to the DB instance, the meta bucket stores the last ID, which is passed to the attached provider, which returns the next ID. This method of declaration allows people to implement external providers if they so choose, but we would have an auto-increment and ULID provider available inside storm.
Make sense?