TrueCloudLab
/
policy-engine
generated from TrueCloudLab/basic
19
0
Fork 8
Code Issues 2 Pull Requests Actions Packages Projects Releases Wiki Activity

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

Merged
fyrchik merged 1 commits 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
aarifullin commented 2024-04-03 11:34:23 +00:00
Collaborator
  • 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 added 2 commits 2024-04-03 11:34:24 +00:00
61ab216d77 [#XX] go.mod: Update frostfs-contract version 
Signed-off-by: Airat Arifullin <aarifullin@yadro.com>
Tests and linters / Tests (1.21) (pull_request) Failing after 1m7s Details
DCO action / DCO (pull_request) Failing after 1m21s Details
Tests and linters / Tests (1.20) (pull_request) Failing after 1m42s Details
Tests and linters / Tests with -race (pull_request) Failing after 1m38s Details
Tests and linters / Staticcheck (pull_request) Failing after 1m34s Details
Tests and linters / Lint (pull_request) Failing after 2m1s Details
0bb48d0a0c [#XX] router: Introduce listByIteratorTraverse option 
* Make defaultChainRouter constructor receive options.
* Deprecate NewDefaultChainRouterWithLocalOverrides.
* If listByIteratorTraverse is set to true, then match a request
  with chains that are traversed by iterator.
* Move parsing a chain from stack item to util package.

Signed-off-by: Airat Arifullin <aarifullin@yadro.com>
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
Poster
Collaborator

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

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
Poster
Collaborator

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
Collaborator

Question: Can we use 694daebb19/commonclient/iterator.go (L14) ?

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
Poster
Collaborator

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
Poster
Collaborator

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
Collaborator

Empty line

Empty line
aarifullin commented 2024-04-08 12:01:31 +00:00
Poster
Collaborator

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
Collaborator

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
Poster
Collaborator

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
Collaborator

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
Poster
Collaborator

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
Poster
Collaborator

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 Label
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 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
There is no content yet.
Delete Branch "aarifullin/policy-engine:feat/list_by_iterator"

Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?