Badgerstore #833

Closed
dstepanov-yadro wants to merge 7 commits from dstepanov-yadro/frostfs-node:feat/badgerstore into master

Another small object store implemenation.

References #555

Another small object store implemenation. References #555
dstepanov-yadro requested review from storage-core-committers 2023-11-29 17:05:19 +00:00
dstepanov-yadro requested review from storage-core-developers 2023-11-29 17:05:19 +00:00
dstepanov-yadro reviewed 2023-11-29 17:08:05 +00:00
@ -0,0 +40,4 @@
//
// Badger uses Dynamic Level Sizes like RocksDB.
// See https://github.com/facebook/rocksdb/blob/v3.11/include/rocksdb/options.h#L366 for explanation.
func defaultCfg() *cfg {
Poster
Collaborator

badger config

badger config
dstepanov-yadro force-pushed feat/badgerstore from f47b48d5b2 to 207820df5e 2023-11-30 13:53:40 +00:00 Compare
aarifullin reviewed 2023-12-01 07:55:02 +00:00
@ -0,0 +6,4 @@
)
func key(add oid.Address) []byte {
res := make([]byte, 64)
Collaborator

Could you move 64 and 32 to constants, please?

Could you move `64` and `32` to constants, please?
Poster
Collaborator

done

done
aarifullin marked this conversation as resolved
dstepanov-yadro force-pushed feat/badgerstore from 207820df5e to 12b80b859d 2023-12-05 08:54:21 +00:00 Compare
dstepanov-yadro force-pushed feat/badgerstore from 12b80b859d to 11f5af27cd 2023-12-07 13:11:46 +00:00 Compare
dstepanov-yadro force-pushed feat/badgerstore from 11f5af27cd to 3e1e0393d7 2023-12-08 12:17:56 +00:00 Compare
dstepanov-yadro force-pushed feat/badgerstore from 3e1e0393d7 to d7c723d310 2023-12-08 12:22:36 +00:00 Compare
dstepanov-yadro force-pushed feat/badgerstore from d7c723d310 to d6ead4d10e 2023-12-08 15:05:07 +00:00 Compare
dstepanov-yadro force-pushed feat/badgerstore from d6ead4d10e to 7ea306d86c 2023-12-08 15:07:33 +00:00 Compare
aarifullin reviewed 2023-12-14 11:24:57 +00:00
@ -0,0 +62,4 @@
}
batch = append(batch, kv)
last = kv.key
if len(batch) == opts.PrefetchSize-1 {
Collaborator

My point is absolutely optional but this check looks like it can be placed to

for it.Seek(last); it.Valid() && len(batch) < opts.PrefetchSize-1; it.Next() {
 /**/
}

:)

My point is absolutely optional but this check looks like it can be placed to ```go for it.Seek(last); it.Valid() && len(batch) < opts.PrefetchSize-1; it.Next() { /**/ } ``` :)
aarifullin marked this conversation as resolved
aarifullin approved these changes 2023-12-14 12:06:32 +00:00
aarifullin left a comment
Collaborator

Brilliant

Brilliant
dstepanov-yadro force-pushed feat/badgerstore from 7ea306d86c to 5b5dc5a50d 2023-12-14 13:38:39 +00:00 Compare
aarifullin reviewed 2023-12-14 15:04:22 +00:00
@ -0,0 +62,4 @@
}
batch = append(batch, kv)
last = kv.key
if len(batch) == opts.PrefetchSize-1 {
Collaborator

Hm. Just curious about if this check is really necessary here. The iterator it is initialized with the option opts.PrefetchSize. Isn't it invalidated on itself when it fetches opts.PrefetchSize items?

Docs: iterating-over-keys

Hm. Just curious about if this check is really necessary here. The iterator `it` is initialized with the option `opts.PrefetchSize`. Isn't it invalidated on itself when it fetches `opts.PrefetchSize` items? [Docs: iterating-over-keys ](https://dgraph.io/docs/badger/get-started/#iterating-over-keys)
Poster
Collaborator

This is done so that there are no long transactions. So storage reads one batch within transaction, returns results to invoker, reads another batch within transaction, etc.

This is done so that there are no long transactions. So storage reads one batch within transaction, returns results to invoker, reads another batch within transaction, etc.
aarifullin marked this conversation as resolved
acid-ant approved these changes 2023-12-15 09:55:40 +00:00
@ -0,0 +47,4 @@
return MemTablesCountDefault
}
// CompactorsCount returns `mem_tables_count` value or CompactorsCountDefault.
Collaborator

mem_tables_count -> compactors_count

mem_tables_count -> compactors_count
Poster
Collaborator

fixed

fixed
acid-ant marked this conversation as resolved
@ -0,0 +96,4 @@
to := from + prm.Range.GetLength()
payload := obj.Payload()
if pLen := uint64(len(payload)); to < from || pLen < from || pLen < to {
Collaborator

How about to check this to < from before get?

How about to check this `to < from` before `get`?

I would leave this code as is -- it is encountered in multiple places (basically, every GetRange) and it is not straightforward to do it right (wrong boundary checks can easily lead to panic or OOM), so now we can just copy it once tested.

I would leave this code as is -- it is encountered in multiple places (basically, every GetRange) and it is not straightforward to do it right (wrong boundary checks can easily lead to panic or OOM), so now we can just copy it once tested.
acid-ant marked this conversation as resolved
dstepanov-yadro force-pushed feat/badgerstore from 5b5dc5a50d to 1b6a06e603 2023-12-20 15:13:24 +00:00 Compare
dstepanov-yadro force-pushed feat/badgerstore from 1b6a06e603 to bd2be1f64e 2023-12-27 11:56:05 +00:00 Compare
dstepanov-yadro force-pushed feat/badgerstore from b2b28b3697 to 82bf0cd2c8 2024-01-09 07:02:38 +00:00 Compare
aarifullin approved these changes 2024-01-10 11:05:54 +00:00
acid-ant approved these changes 2024-01-10 11:20:39 +00:00
dstepanov-yadro force-pushed feat/badgerstore from 82bf0cd2c8 to 5399a852ed 2024-01-10 11:45:23 +00:00 Compare
dstepanov-yadro force-pushed feat/badgerstore from 5399a852ed to c02315fc80 2024-01-15 14:09:21 +00:00 Compare
dstepanov-yadro force-pushed feat/badgerstore from c02315fc80 to 1d4732c2a8 2024-01-18 18:51:40 +00:00 Compare
dstepanov-yadro force-pushed feat/badgerstore from 1d4732c2a8 to ffc7b32197 2024-01-26 10:37:19 +00:00 Compare
dstepanov-yadro force-pushed feat/badgerstore from ffc7b32197 to 6ebbada07c 2024-02-13 12:29:15 +00:00 Compare
dstepanov-yadro force-pushed feat/badgerstore from 6ebbada07c to 824251f650 2024-02-14 14:33:12 +00:00 Compare
dstepanov-yadro force-pushed feat/badgerstore from 20195677a5 to 9f7b114d4f 2024-02-15 10:09:09 +00:00 Compare
dstepanov-yadro requested review from fyrchik 2024-02-21 09:21:08 +00:00
aarifullin approved these changes 2024-02-22 11:15:51 +00:00

I will wait until v0.38, then merge

I will wait until v0.38, then merge
dstepanov-yadro force-pushed feat/badgerstore from 9caf0f9a6d to f5e7a7cffb 2024-04-09 13:54:58 +00:00 Compare
dstepanov-yadro force-pushed feat/badgerstore from f5e7a7cffb to fba92a3d61 2024-04-09 14:01:05 +00:00 Compare
dstepanov-yadro force-pushed feat/badgerstore from fba92a3d61 to ec81c1bcfd 2024-04-09 14:09:58 +00:00 Compare
dstepanov-yadro closed this pull request 2024-06-24 13:49:35 +00:00
Some checks failed
DCO action / DCO (pull_request) Successful in 1m57s
Vulncheck / Vulncheck (pull_request) Successful in 4m45s
Build / Build Components (1.21) (pull_request) Successful in 6m18s
Build / Build Components (1.20) (pull_request) Successful in 6m32s
Tests and linters / Staticcheck (pull_request) Successful in 7m17s
Tests and linters / Lint (pull_request) Successful in 7m54s
Tests and linters / Tests (1.21) (pull_request) Successful in 15m21s
Tests and linters / Tests with -race (pull_request) Successful in 15m27s
Tests and linters / Tests (1.20) (pull_request) Successful in 15m54s
Tests and linters / gopls check (pull_request) Failing after 1m59s

Pull request closed

Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#833
There is no content yet.