router: Make defaultChainRouter match a request by listing chains with iterator #62

Merged
fyrchik merged 1 commit from aarifullin/policy-engine:feat/list_by_iterator into master 2024-04-26 06:20:45 +00:00
Member
  • Make NewContractStorage receive rpcInvoker - that's necessary to construct an invoker.
  • Use commonclient.ReadIteratorItems in ListMorphRuleChains for ContractStorage/ContractStorageReader.
* Make `NewContractStorage` receive `rpcInvoker` - that's necessary to construct an invoker. * Use `commonclient.ReadIteratorItems` in `ListMorphRuleChains` for `ContractStorage/ContractStorageReader`.
aarifullin force-pushed feat/list_by_iterator from 0bb48d0a0c to b2c07fb175 2024-04-03 12:12:03 +00:00 Compare
aarifullin force-pushed feat/list_by_iterator from b2c07fb175 to 6a681bbd6d 2024-04-03 12:36:52 +00:00 Compare
aarifullin force-pushed feat/list_by_iterator from 6a681bbd6d to f1b895d49d 2024-04-03 12:48:23 +00:00 Compare
aarifullin requested review from storage-core-committers 2024-04-05 09:06:09 +00:00
aarifullin requested review from storage-core-developers 2024-04-05 09:06:10 +00:00
aarifullin changed title from WIP: router: Make defaultChainRouter match a request by listing chains with iterator to router: Make defaultChainRouter match a request by listing chains with iterator 2024-04-05 09:06:16 +00:00
aarifullin added the
blocked
label 2024-04-05 09:06:28 +00:00
Author
Member

PR is waiting for frostfs-contract#84

PR is waiting for [frostfs-contract#84](https://git.frostfs.info/TrueCloudLab/frostfs-contract/pulls/84)
aarifullin force-pushed feat/list_by_iterator from f1b895d49d to 9fadbce609 2024-04-05 10:55:06 +00:00 Compare
fyrchik requested changes 2024-04-05 11:24:08 +00:00
@ -105,3 +110,3 @@
}
items, err := s.contractInterface.ListChainsByPrefix(big.NewInt(int64(kind)), target.Name, []byte(name))
sid, it, err := s.contractInterface.IteratorChainsByPrefix(big.NewInt(int64(kind)), target.Name, []byte(name))
Owner

This will be slow, we should try using prefetch script.

This will be slow, we should try using prefetch script.
Author
Member

I have introduced iteration with prefetch script. Please, check this out!

I have introduced iteration with prefetch script. Please, check this out!
aarifullin changed title from router: Make defaultChainRouter match a request by listing chains with iterator to WIP: router: Make defaultChainRouter match a request by listing chains with iterator 2024-04-05 13:15:03 +00:00
aarifullin changed title from WIP: router: Make defaultChainRouter match a request by listing chains with iterator to WIP: router: Make defaultChainRouter match a request by listing chains with iterator 2024-04-05 13:15:08 +00:00
aarifullin force-pushed feat/list_by_iterator from 9fadbce609 to 74490eb4c0 2024-04-05 15:04:12 +00:00 Compare
aarifullin force-pushed feat/list_by_iterator from 74490eb4c0 to ea9641f966 2024-04-05 15:55:41 +00:00 Compare
aarifullin force-pushed feat/list_by_iterator from ea9641f966 to 3ea27b6341 2024-04-05 16:11:32 +00:00 Compare
aarifullin changed title from WIP: router: Make defaultChainRouter match a request by listing chains with iterator to router: Make defaultChainRouter match a request by listing chains with iterator 2024-04-05 16:13:22 +00:00
dkirillov reviewed 2024-04-08 06:26:30 +00:00
@ -112,0 +143,4 @@
return chains, nil
}
script, err := smartcontract.CreateCallAndPrefetchIteratorScript(hash, method, batchSize, params...)
Member

Question: Can we use

func ReadIteratorItems(inv Invoker, batchSize int, contract util.Uint160, method string, params ...any) ([]stackitem.Item, error) {

?

Question: Can we use https://git.frostfs.info/TrueCloudLab/frostfs-contract/src/commit/694daebb1928ffc4eb54e314933dfafe121dad93/commonclient/iterator.go#L14 ?
Author
Member

Oh, that's a good find! I am going to check if I can use this, thank you!

Oh, that's a good find! I am going to check if I can use this, thank you!
Author
Member

That worked out, thanks!

That worked out, thanks!
dkirillov marked this conversation as resolved
aarifullin changed title from router: Make defaultChainRouter match a request by listing chains with iterator to WIP: router: Make defaultChainRouter match a request by listing chains with iterator 2024-04-08 09:16:02 +00:00
aarifullin force-pushed feat/list_by_iterator from 3ea27b6341 to 84b8b6952a 2024-04-08 10:09:07 +00:00 Compare
aarifullin force-pushed feat/list_by_iterator from 84b8b6952a to 75134a62d7 2024-04-08 10:23:00 +00:00 Compare
aarifullin removed the
blocked
label 2024-04-08 10:24:02 +00:00
aarifullin changed title from WIP: router: Make defaultChainRouter match a request by listing chains with iterator to router: Make defaultChainRouter match a request by listing chains with iterator 2024-04-08 10:24:10 +00:00
aarifullin force-pushed feat/list_by_iterator from 75134a62d7 to a25b632946 2024-04-08 10:31:37 +00:00 Compare
aarifullin requested review from fyrchik 2024-04-08 10:35:25 +00:00
dkirillov reviewed 2024-04-08 11:47:34 +00:00
@ -15,6 +16,8 @@ import (
"github.com/nspcc-dev/neo-go/pkg/rpcclient/actor"
"github.com/nspcc-dev/neo-go/pkg/util"
"github.com/nspcc-dev/neo-go/pkg/vm/stackitem"
Member

Empty line

Empty line
Author
Member

fixed

fixed
aarifullin marked this conversation as resolved
@ -39,3 +50,3 @@
var _ engine.MorphRuleChainStorageReader = (*ContractStorageReader)(nil)
func NewContractStorage(actor client.Actor, contract util.Uint160) *ContractStorage {
func NewContractStorage(actor client.Actor, rpcInvoker neoinvoker.RPCInvoke, contract util.Uint160) *ContractStorage {
Member

Do we really need pass actor and invoker separately? Maybe we can pass just more general actor?

Do we really need pass actor and invoker separately? Maybe we can pass just more general actor?
Author
Member

It is a good point. Actually, I've already tried many ways to generilize that and here is a final solution:

type ContractStorageActor interface {
	client.Actor
	GetRPCInvoker() neoinvoker.RPCInvoke
}

type ContractStorageInvoker interface {
	client.Invoker
	GetRPCInvoker() neoinvoker.RPCInvoke
}
  1. I cannot unite client.Actor and neoinvoker.RPCInvoke in one interface, because they are intersected by method TraverseIterator, TerminateSession (duplicate method error)
  2. Getter is needed because client, who's going to implement these interfaces, must guarantee that he returns valid Invoker that uses working connection (basically, websocket connection)
It is a good point. Actually, I've already tried many ways to generilize that and here is a final solution: ```go type ContractStorageActor interface { client.Actor GetRPCInvoker() neoinvoker.RPCInvoke } type ContractStorageInvoker interface { client.Invoker GetRPCInvoker() neoinvoker.RPCInvoke } ``` 1. I cannot unite `client.Actor` and `neoinvoker.RPCInvoke` in one interface, because they are intersected by method `TraverseIterator`, `TerminateSession` (`duplicate method` **error**) 2. Getter is needed because client, who's going to implement these interfaces, must guarantee that he returns valid `Invoker` that uses working connection (basically, websocket connection)
dkirillov marked this conversation as resolved
aarifullin force-pushed feat/list_by_iterator from a25b632946 to 348786d61c 2024-04-08 12:00:35 +00:00 Compare
aarifullin force-pushed feat/list_by_iterator from 348786d61c to afb37f8f61 2024-04-08 12:01:29 +00:00 Compare
aarifullin force-pushed feat/list_by_iterator from afb37f8f61 to c8d06ac8d1 2024-04-08 12:05:02 +00:00 Compare
aarifullin force-pushed feat/list_by_iterator from c8d06ac8d1 to 4cbf51f897 2024-04-08 12:24:02 +00:00 Compare
aarifullin force-pushed feat/list_by_iterator from 4cbf51f897 to 6abac0f5f6 2024-04-08 12:49:52 +00:00 Compare
aarifullin force-pushed feat/list_by_iterator from 6abac0f5f6 to 1c29e2f90a 2024-04-08 12:50:54 +00:00 Compare
aarifullin force-pushed feat/list_by_iterator from 1c29e2f90a to 0b907e7eb2 2024-04-08 12:52:37 +00:00 Compare
dkirillov approved these changes 2024-04-09 06:12:28 +00:00
acid-ant approved these changes 2024-04-09 06:21:57 +00:00
dkirillov reviewed 2024-04-09 11:02:35 +00:00
@ -110,2 +146,4 @@
big.NewInt(int64(kind)), target.Name, []byte(name),
}
items, err := commonclient.ReadIteratorItems(inv, batchSize, hash, method, params...)
Member

Inside this method we don't explicitly terminate session so it can lead to unwrap session iterator: Internal error (-32603) - max session capacity reached. Maybe we can update this method in contract first?

Inside this method we don't explicitly terminate session so it can lead to `unwrap session iterator: Internal error (-32603) - max session capacity reached`. Maybe we can update this method in contract first?
Author
Member

You're absolutely correct. I'll fix this in frostfs-contract

You're absolutely correct. I'll fix this in `frostfs-contract`
Author
Member

go.mod has been updated with TrueCloudLab/frostfs-contract#85

`go.mod` has been updated with https://git.frostfs.info/TrueCloudLab/frostfs-contract/pulls/85
dkirillov marked this conversation as resolved
aarifullin added the
blocked
label 2024-04-09 11:12:10 +00:00
aarifullin force-pushed feat/list_by_iterator from 0b907e7eb2 to 4acfe0e62f 2024-04-09 11:24:11 +00:00 Compare
aarifullin removed the
blocked
label 2024-04-09 11:25:09 +00:00
aarifullin requested review from dkirillov 2024-04-09 11:25:16 +00:00
aarifullin requested review from acid-ant 2024-04-09 11:25:17 +00:00
acid-ant approved these changes 2024-04-09 11:33:10 +00:00
dkirillov approved these changes 2024-04-09 13:04:59 +00:00
fyrchik merged commit c539728641 into master 2024-04-26 06:20:45 +00:00
Sign in to join this conversation.
No description provided.