frostfsid: Add GetGroupByName method #76

mbiryukova · 2024-02-01T13:05:50Z

mbiryukova commented

2024-02-01 13:05:50 +00:00

No description provided.

mbiryukova self-assigned this 2024-02-01 13:05:50 +00:00

mbiryukova force-pushed feature/get_group_by_name from e2b179d31b to 55a4163951

2024-02-01 13:06:38 +00:00

Compare

mbiryukova requested review from storage-core-committers 2024-02-01 13:11:09 +00:00

mbiryukova requested review from storage-core-developers 2024-02-01 13:11:12 +00:00

mbiryukova requested review from storage-services-committers 2024-02-01 13:11:37 +00:00

mbiryukova requested review from storage-services-developers 2024-02-01 13:11:38 +00:00

dkirillov approved these changes 2024-02-01 14:07:42 +00:00

fyrchik approved these changes 2024-02-01 17:44:28 +00:00

frostfsid/frostfsid_contract.go

					
				@ -586,2 +586,4 @@

				}

				func GetGroupByName(ns, name string) Group {

					groupID := GetGroupIDByName(ns, name)

fyrchik commented

2024-02-01 17:44:25 +00:00

Wanted to mention that this way we call GetReadOnlyContext twice.
But then realized that CALL opcode seems to be even more expensive than this interop (1<<4 vs 1<<9).

Wanted to mention that this way we call `GetReadOnlyContext` twice. But then realized that CALL opcode seems to be even more expensive than this interop (1<<4 vs 1<<9).

fyrchik marked this conversation as resolved

fyrchik commented

2024-02-01 17:47:50 +00:00

What is the motivation, though? We have GetGroupByName, GetGroup and GetGroudIDByName. The first seems to be a composition of the second and the third. Do we need this as a single method in contract? Is doesn't seem hard to make a client wrapper. My concern is contract size (frostfs-adm packs multiple contracts in a single transaction) and the interface we need to support in future.
cc @alexvanin

What is the motivation, though? We have `GetGroupByName`, `GetGroup` and `GetGroudIDByName`. The first seems to be a composition of the second and the third. Do we need this as a single method in contract? Is doesn't seem hard to make a client wrapper. My concern is contract size (frostfs-adm packs multiple contracts in a single transaction) and the interface we need to support in future. cc @alexvanin

dkirillov commented

2024-02-02 06:41:06 +00:00

The initial motivation was reducing number of contract invocations. So we have got GetSubjectByName first, now we make contract more consistent. But probably we can form two contract invocation in one transaction on client

The initial motivation was reducing number of contract invocations. So we have got `GetSubjectByName` first, now we make contract more consistent. But probably we can form two contract invocation in one transaction on client

acid-ant approved these changes 2024-02-02 07:55:26 +00:00

fyrchik merged commit 55a4163951 into master

2024-02-02 08:46:32 +00:00

fyrchik referenced this pull request from a commit

2024-02-02 08:46:33 +00:00

[#76] frostfsid: Add GetGroupByName method

Sign in to join this conversation.

No reviewers