NeoFS block storage: add uploading commands #3582

AliceInHunterland · 2024-09-11T06:13:07Z

example usage:

./bin/neo-go util upload-bin --cid 9iVfUg8aDHKjPC4LhQXEkVUM4HDkR7UCXYLs8NQwYfSG --wallet-config ./wallet-config.yml --block-attribute "test_final2345678901234" --index-attribute "oid-index" --rpc-endpoint https://rpc.t5.n3.nspcc.ru:20331 -fsr st1.t5.fs.neo.org:8080

codecov · 2024-09-11T06:20:51Z

Codecov Report

Attention: Patch coverage is 4.56140% with 272 lines in your changes missing coverage. Please review.

Project coverage is 84.62%. Comparing base (9a38360) to head (0a4e1d1).
Report is 9 commits behind head on master.

Files with missing lines	Patch %	Lines
cli/util/uploader.go	0.00%	265 Missing ⚠️
cli/util/convert.go	72.22%	4 Missing and 1 partial ⚠️
pkg/services/oracle/neofs/neofs.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3582      +/-   ##
==========================================
- Coverage   85.26%   84.62%   -0.65%     
==========================================
  Files         333      334       +1     
  Lines       39005    39292     +287     
==========================================
- Hits        33256    33249       -7     
- Misses       4177     4475     +298     
+ Partials     1572     1568       -4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

AliceInHunterland · 2024-09-11T06:22:54Z

@AnnaShaleva what do you think about adding more commands to neo-go cli?
The current pipeline of updating blocks in the container is as follows:

dump-bin
dump-bin-put
dump-generate-index-file
uploading index files (not a command)

cli/server/dump_bin_put.go

cli/server/server.go

cli/server/dump_bin_put.go

cli/util/convert.go

AnnaShaleva · 2024-10-03T07:08:36Z

cli/util/uploader.go

+	indexFileAttribute := ctx.String("index-attribute")
+	acc, _, err := options.GetAccFromContext(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to load account: %w", err)


It should be a proper CLI exit error.

We need unit-tests at least for error cases of this command. As mach cases should be covered as possible without access to NeoFS nodes (NeoGo RPC is included into Executor and hence may be used for tests).

cli/util/uploader.go

AnnaShaleva · 2024-10-03T10:16:52Z

cli/util/uploader.go

+	expectedIndexCount := currentHeight / uint(indexFileSize)
+
+	if existingIndexCount == expectedIndexCount {
+		return nil


Write that index files are up-to-date.

...Existing: %d, expected:...

cli/util/uploader.go

AnnaShaleva · 2024-10-03T10:27:09Z

cli/util/uploader.go

+				defer wg.Done()
+				defer func() { <-workerPool }()
+
+				oidBlock, err := searchBlockOID(ctx, clientSDK, account, containerID, blockAttributeKey, index)


It's still there; we can't use Search request for every block, it's too time-consuming, imagine we have to perform 128K Search requests.

Time taken for index file generation with searches: 17m3.919291917s
Time taken for index file generation with getHeader: 14m57.008241s

I've expected a bigger difference, btw. Still, that's it.

cli/util/uploader.go

AnnaShaleva · 2024-10-04T09:03:38Z

BTW, linter is failing.

AliceInHunterland · 2024-10-05T19:20:30Z

706 seconds / 1000 blocks = 0.706 seconds/block. 11m46s for 1k blocks with slicer version

394 seconds / 1000 blocks = 0.394 seconds/block. 6m34s for 1k blocks with pool (it still a month)

with something like:

numWorkers := 5
	blockChan := make(chan int, numWorkers)
	errChan := make(chan error, numWorkers)
	var wg sync.WaitGroup

	for i := 0; i < numWorkers; i++ {
		wg.Add(1)
		go func() {
			defer wg.Done()
			for blockIndex := range blockChan {
                              block, err := rpc.GetBlockByIndex(uint32(blockIndex))
                              ....}()
         }
	go func() {
		for blockIndex := maxBlockIndex + 1; blockIndex <= int(currentBlockHeight); blockIndex++ {
			blockChan <- blockIndex
		}
		close(blockChan)
	}()
	
	go func() {
		wg.Wait()
		close(errChan)
	}()

74 seconds / 1000 blocks = 0.074 seconds/block. 1m14s for 1k blocks (~5 days)
producer(4)-consumer(6) 52 seconds
producer(10)-consumer(20) 15 seconds,but:

[2024-10-06T01:47:40+03:00] Successfully uploaded block: 442000
panic: runtime error: index out of range [-1]

goroutine 247 [running]:
math/rand.(*rngSource).Uint64(...)
        math/rand/rng.go:249
math/rand.(*rngSource).Int63(0x1400048d6a8?)
        math/rand/rng.go:234 +0x98
math/rand.(*Rand).Int63(...)
        math/rand/rand.go:96
math/rand.(*Rand).Int31(...)
        math/rand/rand.go:110
math/rand.(*Rand).Int31n(0x14000040750?, 0x48d7a0?)
        math/rand/rand.go:142 +0x8c
math/rand.(*Rand).Intn(0x10?, 0x10?)
        math/rand/rand.go:183 +0x30
github.com/nspcc-dev/neofs-sdk-go/pool.(*sampler).Next(0x140001cf3c0)
        github.com/nspcc-dev/neofs-sdk-go@v1.0.0-rc.12/pool/sampler.go:63 +0x2c
github.com/nspcc-dev/neofs-sdk-go/pool.(*innerPool).connection(0x140001cf400)
        github.com/nspcc-dev/neofs-sdk-go@v1.0.0-rc.12/pool/pool.go:988 +0x1a0
github.com/nspcc-dev/neofs-sdk-go/pool.(*Pool).connection(0x60?)
        github.com/nspcc-dev/neofs-sdk-go@v1.0.0-rc.12/pool/pool.go:967 +0x58
github.com/nspcc-dev/neofs-sdk-go/pool.(*Pool).sdkClient(0xad8f60c33a130b3?)
        github.com/nspcc-dev/neofs-sdk-go@v1.0.0-rc.12/pool/pool.go:1031 +0x1c
github.com/nspcc-dev/neofs-sdk-go/pool.(*Pool).ObjectPutInit(0x1400048da88, {0x101e0dec0, 0x1027af240}, {0x0, 0x0, 0x1400422a2a0, {0x0, 0x0, 0x0}}, {0x101e0e400, ...}, ...)
        github.com/nspcc-dev/neofs-sdk-go@v1.0.0-rc.12/pool/object.go:35 +0xa4
github.com/nspcc-dev/neo-go/cli/util.uploadBlock({0x101e0dec0, 0x1027af240}, {{0x14000192180, 0x1, 0x1}, {0x101e0e400, 0x140003f53b0}, 0x140002fb150, 0x14000436a80, 0x1400004b910, ...}, ...)
        github.com/nspcc-dev/neo-go/cli/util/uploader.go:401 +0x11c
github.com/nspcc-dev/neo-go/cli/util.uploadBin.func3()
        github.com/nspcc-dev/neo-go/cli/util/uploader.go:162 +0x6bc
created by github.com/nspcc-dev/neo-go/cli/util.uploadBin in goroutine 1
        github.com/nspcc-dev/neo-go/cli/util/uploader.go:145 +0xb48
(base) ekaterinapavlova@MacBook-Air-4 neo-go %

./bin/neo-go util upload-bin --cid 9iVfUg8aDHKjPC4LhQXEkVUM4HDkR7UCXYLs8NQwYfSG --wallet-config ./wallet-config.yml --block-attribute "ekt_block_pool_pr_cons" --index-attribute "oid-index" --rpc-endpoint https://rpc.t5.n3.nspcc.ru:20331 -fsr st1.t5.fs.neo.org:8080 -fsr st2.t5.fs.neo.org:8080 -fsr st3.t5.fs.neo.org:8080 -fsr st4.t5.fs.neo.org:8080

I haven't managed to repeat panic. successfully uploaded 1447000 blocks to testnet.

AliceInHunterland · 2024-10-06T09:17:08Z

cli/util/uploader.go

+		}
+
+		attrs := []object.Attribute{
+			*object.NewAttribute("block", strconv.Itoa(blockIndex)),                // Block index


"block" to attr

Yep, we have to use user-provided value.

AnnaShaleva · 2024-10-07T09:55:28Z

cli/util/convert.go

@@ -109,6 +147,13 @@ func NewCommands() []*cli.Command {
 						},
 					},
 				},
+				{
+					Name:      "upload-bin",
+					Usage:     "Fetch blocks from RPC node and upload it to the NeoFS container",


s/upload it/upload them?

AnnaShaleva · 2024-10-07T10:00:41Z

cli/util/convert.go

+				{
+					Name:      "upload-bin",
+					Usage:     "Fetch blocks from RPC node and upload it to the NeoFS container",
+					UsageText: "neo-go util upload-bin --fs-rpc-endpoint address --container cid --block-attribute string --index-attribute string --rpc-endpoint address --wallet wallet [--wallet-config config] [--address address]",


--fs-rpc-endpoint <address1>[,<address2>[...]]

--block-attribute block
--index-attribute index
--rpc-endpoint <node> [--timeout <time>]

And while we're not yet have mainnet uploads, let's rename index file object attribute from oid to index (documentation and node config should be fixed).

AnnaShaleva · 2024-10-07T10:06:57Z

cli/util/convert.go

+					Usage:     "Fetch blocks from RPC node and upload it to the NeoFS container",
+					UsageText: "neo-go util upload-bin --fs-rpc-endpoint address --container cid --block-attribute string --index-attribute string --rpc-endpoint address --wallet wallet [--wallet-config config] [--address address]",
+					Action:    uploadBin,
+					Flags:     putFlags,


Rename putFlags to uploadBinFlags.

cli/util/uploader.go

With ObjectSearchInitter ObjectSearch can be done both NeoFS SDK client or pool. Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>

Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>

This command is used for keeping container with blocks for blockfetcher updated. Close #3578 Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io> README: add archival notice (#520) People are likely to find this repo and we better have some directions for them. Signed-off-by: Roman Khimov <roman@nspcc.ru> Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>

AliceInHunterland · 2024-10-09T06:11:43Z

cli/util/uploader.go

+	producerWg.Add(numProducers)
+
+	// Retry function with exponential backoff
+	retry := func(action func() error) error {


Should we save it? Maybe yes? I like it. With it, timeouts are rare occasions.

Let's keep it. Could you please point to the part of code that has a lot of timeout failures?

AliceInHunterland · 2024-10-09T06:16:17Z

cli/util/uploader.go

+			*object.NewAttribute(attributeKey, strconv.Itoa(int(i))),
+			*object.NewAttribute("size", strconv.Itoa(int(indexFileSize))),
+			*object.NewAttribute("timestamp", strconv.FormatInt(time.Now().Unix(), 10)),
+			*object.NewAttribute("block-attribute", blockAttributeKey),


@AnnaShaleva what do you think about adding two more attributes to index files? timestamp seems to me now useless -NeoFS will put it by itself. But block-attribute could be helpful for debagging and manual search.

For timestamp - OK, remove it from this list. For block-attribute: I'd say it's a bit excessive for the real usage because we always have block-attribute in the node settings.

AliceInHunterland · 2024-10-09T06:17:04Z

cli/util/convert.go

@@ -6,6 +6,7 @@ import (
 	"fmt"


neo-vm will be removed from this commit

and commit message should be adjusted

AliceInHunterland · 2024-10-09T06:38:47Z

docs/neofs-blockstorage.md

@@ -16,12 +16,12 @@ attributes:
 - primary node index (`primary:0`)
 - block hash in the LE form (`hash:5412a781caf278c0736556c0e544c7cfdbb6e3c62ae221ef53646be89364566b`)
 - previous block hash in the LE form (`prevHash:3654a054d82a8178c7dfacecc2c57282e23468a42ee407f14506368afe22d929`)
- - millisecond-precision block timestamp (`time:1627894840919`)
+ - millisecond-precision block timestamp (`timestamp:1627894840919`)


commit message

AnnaShaleva

Very good indeed! A couple of logic fixes are still needed, but overall looks much better.

AnnaShaleva · 2024-10-09T08:13:42Z

pkg/services/oracle/neofs/neofs.go

 // ObjectSearch returns a list of object IDs from the provided container.
-func ObjectSearch(ctx context.Context, c *client.Client, priv *keys.PrivateKey, containerIDStr string, prm client.PrmObjectSearch) ([]oid.ID, error) {
+func ObjectSearch(ctx context.Context, initter ObjectSearchInitter, priv *keys.PrivateKey, containerIDStr string, prm client.PrmObjectSearch) ([]oid.ID, error) {


The code itself is perfect, but regarding commit message: neofs: add pool for searching it seems not to be matching the commit content, thus please rephrase.

AnnaShaleva · 2024-10-09T09:27:07Z

cli/util/uploader.go

+	fmt.Fprintln(ctx.App.Writer, "Chain block height:", currentBlockHeight)
+
+	signer := user.NewAutoIDSignerRFC6979(acc.PrivateKey().PrivateKey)
+


Useless blank line.

AnnaShaleva · 2024-10-09T09:56:59Z

cli/util/uploader.go

+// fetchMaxBlockIndex searches the container for the maximum block index.
+func fetchMaxBlockIndex(ctx context.Context, p *pool.Pool, containerID cid.ID, priv *keys.PrivateKey, attributeKey string) (int, error) {
+	var wg sync.WaitGroup
+	height := 0


Good, but: why not to search starting from the current height? Uploader can (and should) guarantee that only single latest batch may be incomplete; older batches are guaranteed to be completed. If currently uploader can't guarantee that, then uploader algorithm should be fixed.

AnnaShaleva · 2024-10-09T10:03:49Z

cli/util/uploader.go

+			if res.numOIDs < searchBatchSize {
+				if res.startIndex == 0 && res.numOIDs == 0 {
+					return -1, nil
+				}
+				// Return the start index of the first incomplete batch
+				return res.startIndex, nil


Please, check it and fix the code if it's true: we'll try to re-upload incomplete batch starting from block with index res.startIndex; then we should not return -1 because there's no block with index -1. Genesis block is not special anymore, we should remove the following part:

if res.startIndex == 0 && res.numOIDs == 0 { return -1, nil }

AnnaShaleva · 2024-10-09T10:08:08Z

cli/util/uploader.go

+}
+
+// fetchMaxBlockIndex searches the container for the maximum block index.
+func fetchMaxBlockIndex(ctx context.Context, p *pool.Pool, containerID cid.ID, priv *keys.PrivateKey, attributeKey string) (int, error) {


And since the algorithm was changed, let's rename this function to fetchLatestMissingBlockIndex. And adjust documentation, specify that it's the index of the first block in the latest incomplete batch.

AnnaShaleva · 2024-10-09T11:11:35Z

cli/util/uploader.go

+
+	var (
+		processedCounter atomic.Int32
+		errCh            = make(chan error, 1)


Make it non-buffered.

AnnaShaleva · 2024-10-09T11:13:36Z

cli/util/uploader.go

+		}
+		fmt.Fprintf(ctx.App.Writer, "Uploaded index file %d\n", i)
+	}
+	close(oidCh)


Move this close to a deferred statement.

AnnaShaleva · 2024-10-09T11:21:37Z

cli/util/uploader.go

+	hdr.SetVersion(v)
+
+	checksum.Calculate(&ch, checksum.TZ, objData)
+	hdr.SetPayloadHomomorphicHash(ch)


We need to calculate/set it only if it's enabled in the network.

AnnaShaleva · 2024-10-09T11:22:38Z

cli/util/uploader.go

+func getBlockIndex(header object.Object, attribute string) (int, error) {
+	for _, attr := range header.UserAttributes() {
+		if attr.Key() == attribute {
+			return strconv.Atoi(attr.Value())


Very nice, much better way.

AnnaShaleva · 2024-10-09T11:23:19Z

go.sum

+github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migciow=
+github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM=
+github.com/Microsoft/hcsshim v0.11.4 h1:68vKo2VN8DE9AdN4tnkWnmdhqdbpUFM8OF3Airm7fz8=
+github.com/Microsoft/hcsshim v0.11.4/go.mod h1:smjE4dvqPX9Zldna+t5FG3rnoHhaB7QYxPRqGcpAD9w=


Something has happened with go modules and VM git submodule, please, remove these changes from the PR.

AliceInHunterland force-pushed the block-fetcher-commands branch from 8e1d6b3 to 64f6c7a Compare September 13, 2024 20:59

AliceInHunterland force-pushed the block-fetcher-commands branch 6 times, most recently from 2938069 to 13e075a Compare September 27, 2024 10:09

AnnaShaleva reviewed Oct 1, 2024

View reviewed changes

AliceInHunterland force-pushed the block-fetcher-commands branch from 13e075a to a6dc319 Compare October 1, 2024 15:04

AliceInHunterland marked this pull request as ready for review October 1, 2024 15:06

AliceInHunterland requested a review from roman-khimov as a code owner October 1, 2024 15:06

AliceInHunterland requested a review from AnnaShaleva October 1, 2024 15:08

AliceInHunterland force-pushed the block-fetcher-commands branch from a6dc319 to e5601ee Compare October 1, 2024 15:09

AnnaShaleva reviewed Oct 3, 2024

View reviewed changes

cli/util/uploader.go Outdated Show resolved Hide resolved

AliceInHunterland force-pushed the block-fetcher-commands branch from e5601ee to 1ddbbda Compare October 5, 2024 19:21

AliceInHunterland commented Oct 6, 2024

View reviewed changes

AliceInHunterland requested a review from AnnaShaleva October 7, 2024 07:15

AliceInHunterland mentioned this pull request Oct 7, 2024

Pool unexpected panic nspcc-dev/neofs-sdk-go#631

Open

AnnaShaleva reviewed Oct 7, 2024

View reviewed changes

AliceInHunterland added 3 commits October 9, 2024 00:54

neofs: add pool for searching

c635815

With ObjectSearchInitter ObjectSearch can be done both NeoFS SDK client or pool. Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>

docs: adjust default IndexFileAttribute

3c0605b

Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>

AliceInHunterland force-pushed the block-fetcher-commands branch from 1ddbbda to 0a4e1d1 Compare October 9, 2024 06:09

AliceInHunterland commented Oct 9, 2024

View reviewed changes

AliceInHunterland requested a review from AnnaShaleva October 9, 2024 06:39

AnnaShaleva reviewed Oct 9, 2024

View reviewed changes

		fmt.Fprintln(ctx.App.Writer, "Chain block height:", currentBlockHeight)

		signer := user.NewAutoIDSignerRFC6979(acc.PrivateKey().PrivateKey)

NeoFS block storage: add uploading commands #3582

Are you sure you want to change the base?

NeoFS block storage: add uploading commands #3582

Conversation

AliceInHunterland commented Sep 11, 2024 • edited Loading

codecov bot commented Sep 11, 2024 • edited Loading

Codecov Report

AliceInHunterland commented Sep 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AnnaShaleva commented Oct 4, 2024

AliceInHunterland commented Oct 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AliceInHunterland Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AliceInHunterland Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

AnnaShaleva Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AnnaShaleva left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AliceInHunterland commented Sep 11, 2024 •

edited

Loading

codecov bot commented Sep 11, 2024 •

edited

Loading

AliceInHunterland commented Oct 5, 2024 •

edited

Loading

AliceInHunterland Oct 9, 2024 •

edited

Loading

AliceInHunterland Oct 9, 2024 •

edited

Loading

AnnaShaleva Oct 9, 2024 •

edited

Loading