feat: unreserve on-demand #2071

acud · 2021-06-12T18:41:13Z

#2040

This change is

TODO:

~~on batchstore reset we should delete all the fifo queue items~~ (already implicitly done by the Reset method)
~~retrieval protocol has to check postage stamp validity and force cache in case the stamp is invalid~~

zelig

Reviewed 11 of 11 files at r1.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @acud)

pkg/localstore/gc.go, line 47 at r1 (raw file):

	// not to overwhelm the cache with too many chunks which it will flush
	// anyway.
	reserveCollectionRatio = 0.5

this should be more like 10% max

pkg/localstore/gc_test.go, line 1017 at r1 (raw file):

	})
	db.SetUnreserveFunc(unres)
	fmt.Println(1)

debug code

pkg/postage/batchstore/reserve.go, line 52 at r1 (raw file):

var Capacity = exp2(22)

const unreservePage = 50 // how many entries to ask for deletion

what is the purpose of this?

pkg/postage/batchstore/reserve.go, line 73 at r1 (raw file):

// dequeued by the localstore once the storage fills up.
func (s *store) unreserve(b *postage.Batch, radius uint8) error {
	c, err := s.getQueueCardinality()

this need not be peristed in fact. just make sure its a value on s atomically incremented

pkg/postage/batchstore/reserve.go, line 88 at r1 (raw file):

}

func (s *store) Unreserve() error {

not sure why this is not an actual iterator. Make it an iterator and you can

eliminate the page constant,
get rid of unreserveFunc and its settter
take onset as a parameter, then you can move pruning to an async routine

pkg/postage/batchstore/reserve.go, line 106 at r1 (raw file):

		entries[i] = string(key)
		i++
		if i == unreservePage {

return i==unreserePage, nil

pkg/postage/batchstore/reserve.go, line 114 at r1 (raw file):

		return err
	}
	for _, v := range entries {

this better be asynchronous and done on demand

pkg/postage/batchstore/reserve.go, line 366 at r1 (raw file):

}

func (s *store) getQueueCardinality() (val uint64, err error) {

just needs to be initialised as the the topmost index

pkg/postage/batchstore/reserve.go, line 375 at r1 (raw file):

}

func (s *store) putQueueCardinality(val uint64) error {

same

pkg/postage/batchstore/reserve_test.go, line 839 at r1 (raw file):

}

func TestUnreserveItemSequence(t *testing.T) {

what is this testing. seems a bit complex

zelig · 2021-06-13T00:11:53Z

mock batchstore now fails on interface (no Unreserve)
Unreserve function (hopefully iterator) can take onset and bucketDepth

TODO:

let r be radius part of latest unreserveItem picked up by localstore via iterator call
r+1 to be saved as storage radius within reserveState to expose it through the API for price oracle?
remember cursor sequence count (at constructor call initialise from the queue), so that it is not taken from level db at every call

zelig

Reviewed 14 of 14 files at r2.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @acud)

pkg/postage/interface.go, line 37 at r2 (raw file):

	GetReserveState() *ReserveState
	SetRadiusSetter(RadiusSetter)
	Unreserve(UnreserveIteratorFn) error

this should be a nother interface

pkg/postage/batchstore/reserve.go, line 71 at r2 (raw file):

// dequeued by the localstore once the storage fills up.
func (s *store) unreserve(b []byte, radius uint8) error {
	c, err := s.getQueueCardinality()

this can be optimised

pkg/postage/batchstore/mock/store.go, line 143 at r2 (raw file):

	return rs
}
func (bs *BatchStore) Unreserve(_ postage.UnreserveIteratorFn) {

not needed if on a different interface

zelig · 2021-06-13T13:04:10Z

LGTM when tests pass

zelig

Reviewed 6 of 8 files at r3, 12 of 12 files at r4.
Reviewable status: all files reviewed, 17 unresolved discussions (waiting on @acud)

pkg/localstore/reserve_test.go, line 302 at r4 (raw file):

			t.Fatal(err)
		}
		mtx.Lock()

this lock is not needed right?

pkg/localstore/reserve_test.go, line 389 at r4 (raw file):

	unres := func(f postage.UnreserveIteratorFn) error {
		mtx.Lock()

why lock is needed

pkg/node/node.go, line 334 at r4 (raw file):

	b.p2pHalter = p2ps

	var unreserveFn func([]byte, uint8) (uint64, error)

maybe comment here about this black magic going on to make reciprocal dependency painless

pkg/postage/batchstore/reserve.go, line 60 at r4 (raw file):

	// would like to guarantee that all chunks are stored
	Radius        uint8 `json:"radius"`
	StorageRadius uint8 `json:"storageRadius"`

add comment: the current actual radius of eviction/storage

pkg/postage/batchstore/reserve.go, line 110 at r4 (raw file):

v.Radius != swarm.MaxPO+1

this condition is no longer needed

acud

Reviewable status: all files reviewed, 17 unresolved discussions (waiting on @acud and @zelig)

pkg/localstore/gc.go, line 47 at r1 (raw file):