TrueCloudLab/policy-engine
20
0
Fork 11
generated from TrueCloudLab/basic
Code Issues 5 Pull requests 1 Projects Releases Packages Wiki Activity Actions

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
Conversation 1 Commits 1 Files changed 3 +63 -35
aarifullin commented 2024-04-03 11:34:23 +00:00
Member
Copy link
  • 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
aarifullin commented 2024-04-05 09:06:54 +00:00
Author
Member
Copy link

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
pkg/morph/policy/policy_contract_storage.go Outdated
@ -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))
fyrchik commented 2024-04-05 11:24:06 +00:00
Owner
Copy link

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

This will be slow, we should try using prefetch script.
aarifullin commented 2024-04-05 16:13:50 +00:00
Author
Member
Copy link

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
pkg/morph/policy/policy_contract_storage.go Outdated
@ -112,0 +143,4 @@
return chains, nil
}
script, err := smartcontract.CreateCallAndPrefetchIteratorScript(hash, method, batchSize, params...)
dkirillov commented 2024-04-08 06:26:30 +00:00
Member
Copy link

Question: Can we use

Line 14 in TrueCloudLab/frostfs-contract@694daeb
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 ?
aarifullin commented 2024-04-08 09:15:29 +00:00
Author
Member
Copy link

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!
aarifullin commented 2024-04-08 10:24:22 +00:00
Author
Member
Copy link

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
pkg/morph/policy/policy_contract_storage.go Outdated
@ -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"
dkirillov commented 2024-04-08 11:35:09 +00:00
Member
Copy link

Empty line

Empty line
aarifullin commented 2024-04-08 12:01:31 +00:00
Author
Member
Copy link

fixed

fixed
aarifullin marked this conversation as resolved
pkg/morph/policy/policy_contract_storage.go Outdated
@ -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 {
dkirillov commented 2024-04-08 11:47:28 +00:00
Member
Copy link

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?
aarifullin commented 2024-04-08 12:56:47 +00:00
Author
Member
Copy link

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
pkg/morph/policy/policy_contract_storage.go Outdated
@ -110,2 +146,4 @@
big.NewInt(int64(kind)), target.Name, []byte(name),
}
items, err := commonclient.ReadIteratorItems(inv, batchSize, hash, method, params...)
dkirillov commented 2024-04-09 11:02:35 +00:00
Member
Copy link

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?
aarifullin commented 2024-04-09 11:12:02 +00:00
Author
Member
Copy link

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

You're absolutely correct. I'll fix this in `frostfs-contract`
aarifullin commented 2024-04-09 11:24:58 +00:00
Author
Member
Copy link

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.
Reviewers
No reviewers
fyrchik
acid-ant
dkirillov
TrueCloudLab/storage-core-developers
Labels
Clear labels
  
Infrastructure

Infrastructure, Automation, CI/CD or DevOps related stuff

  
blocked

The issue or PR is blocked by something

  
bug

Something is not working

  
config

Configuration format update or breaking change

  
discussion

Talking about something in order to reach a decision or to exchange ideas

  
documentation

Documentation related

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
go

Language features adoption

  
help wanted

Extra attention is needed

  
internal

   
invalid

Something is wrong

  
kludge

This is a kludge and we know it

  
observability

Related to metrics, tracing and logs

  
perfomance

Perfomance related

  
question

More information is needed

  
refactoring

Refactoring

  
wontfix

This won't be fixed

No labels
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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/policy-engine#62
No description provided.
Delete branch "aarifullin/policy-engine:feat/list_by_iterator"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?