>
> sync.RWMutex works well as an embedded struct field. Locking DB could be
> db.Lock() instead of db.mu.Lock(), and the same could apply to other fields.
I considered this option, but as Jakob mentioned earlier it would make the
Locker interface public.
Maybe consider making errFull or other global error vars public so a caller
> could do handling for more specific error conditions.
Good call.
The benchmark graph seems to only have four samples for each. That may be
> reasonable but I’d generally expect a graph with more detail.
It's taking a really long time to run the benchmark with 100M+ keys, but
I'll update the graphs later.
For the API, why are keys represented as []byte instead of string? Have you
> considered interface{} for keys and values?
I wanted to make the API compatible with other key/value store APIs
(goleveldb, bolt and badgerdb). There is no doubt, supporting interface{}
for keys and values would make pogreb much more user-friendly, I'll
consider implementing that in v2.
Overall this seems like a well written library to me after a quick look.
> There is a decent number of tests and documentation is there.
I appreciate your feedback, thank you Matt!
Regards,
Artem
On Monday, January 8, 2018 at 5:36:22 PM UTC-5, matthe...@gmail.com wrote:
>
> Here’s a code review:
>
> sync.RWMutex works well as an embedded struct field. Locking DB could be
> db.Lock() instead of db.mu.Lock(), and the same could apply to other fields.
>
> type DB struct {
> sync.RWMutex
> …
>
> type Options struct {
> BackgroundSyncInterval time.Duration
> fs.FileSystem
> }
>
> Seeing things like “db.metrics.BucketProbes.Add(1)” or
> “db.data.allocate(bucketSize)” seems excessive for Go; the struct embedding
> shortcuts help with readability.
>
> Your error handling seems done well. Maybe consider making errFull or
> other global error vars public so a caller could do handling for more
> specific error conditions.
>
> The benchmark graph seems to only have four samples for each. That may be
> reasonable but I’d generally expect a graph with more detail.
>
> For the API, why are keys represented as []byte instead of string? Have
> you considered interface{} for keys and values?
>
> The DB Open function documentation should say that a pogreb/fs.OS
> FileSystem is the default.
>
> Overall this seems like a well written library to me after a quick look.
> There is a decent number of tests and documentation is there.
>
> Matt
>
> On Monday, January 8, 2018 at 1:59:14 PM UTC-6, Artem Krylysov wrote:
>>
>> Hello everyone!
>>
>> I made a first release of a new key-value store optimized for fast random
>> lookups https://github.com/akrylysov/pogreb
>>
>> Random lookup performance benchmark results
>> https://akrylysov.github.io/pogreb/read-bench.png
>>
>> I will be happy to receive any feedback.
>>
>> Regards,
>> Artem
>>
>
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
