-
Notifications
You must be signed in to change notification settings - Fork 68
remove id caches #1283
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
remove id caches #1283
Changes from all commits
5989c62
56582bf
4ddbd8e
38f34a6
bca01a1
1db4f15
9acc27c
b9421e6
44c7e9c
955e0a8
7c14a70
41bf7d0
e7953c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ import ( | |
| "github.com/hyperledger-labs/fabric-token-sdk/token/driver" | ||
| idriver "github.com/hyperledger-labs/fabric-token-sdk/token/services/identity/driver" | ||
| "github.com/hyperledger-labs/fabric-token-sdk/token/services/logging" | ||
| cache2 "github.com/hyperledger-labs/fabric-token-sdk/token/services/utils/cache" | ||
| "go.uber.org/zap/zapcore" | ||
| ) | ||
|
|
||
|
|
@@ -73,7 +74,6 @@ type Provider struct { | |
|
|
||
| isMeCache cache[bool] | ||
| signers cache[*SignerEntry] | ||
| verifiers cache[*VerifierEntry] | ||
| } | ||
|
|
||
| // NewProvider creates a new identity provider implementing the driver.IdentityProvider interface. | ||
|
|
@@ -91,9 +91,8 @@ func NewProvider( | |
| enrollmentIDUnmarshaler: enrollmentIDUnmarshaler, | ||
| deserializer: deserializer, | ||
| storage: storage, | ||
| isMeCache: secondcache.NewTyped[bool](5000), | ||
| signers: secondcache.NewTyped[*SignerEntry](5000), | ||
| verifiers: secondcache.NewTyped[*VerifierEntry](5000), | ||
| isMeCache: cache2.NewNoCache[bool](), | ||
| signers: secondcache.NewTyped[*SignerEntry](5), | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -117,13 +116,6 @@ func (p *Provider) RegisterVerifier(ctx context.Context, identity driver.Identit | |
| if v == nil { | ||
| return errors.New("invalid verifier, expected a valid instance") | ||
| } | ||
| idHash := identity.UniqueID() | ||
| entry := &VerifierEntry{Verifier: v} | ||
| if p.Logger.IsEnabledFor(zapcore.DebugLevel) { | ||
| entry.DebugStack = debug.Stack() | ||
| } | ||
| p.verifiers.Add(idHash, entry) | ||
| p.Logger.DebugfContext(ctx, "register verifier to [%s]:[%s]", idHash, logging.Identifier(v)) | ||
| return nil | ||
| } | ||
|
|
||
|
|
@@ -274,6 +266,7 @@ func (p *Provider) getSigner(ctx context.Context, identity driver.Identity, idHa | |
| if err != nil { | ||
| return nil, errors.Wrapf(err, "failed deserializing identity for signer [%s]", identity) | ||
| } | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we said to cache only the identity whose type is x509, no? |
||
| entry = &SignerEntry{Signer: signer} | ||
| if p.Logger.IsEnabledFor(zapcore.DebugLevel) { | ||
| entry.DebugStack = debug.Stack() | ||
|
|
@@ -282,6 +275,7 @@ func (p *Provider) getSigner(ctx context.Context, identity driver.Identity, idHa | |
| if err := p.storage.StoreSignerInfo(ctx, identity, nil); err != nil { | ||
| return nil, errors.Wrap(err, "failed to store entry in storage for the passed signer") | ||
| } | ||
|
|
||
| return entry.Signer, nil | ||
| } | ||
|
|
||
|
|
@@ -331,15 +325,4 @@ func (p *Provider) updateCaches(descriptor *idriver.IdentityDescriptor, alias dr | |
| p.signers.Add(aliasID, entry) | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we are still caching any signer no matter the identity type. This will generate even more contention on the signers cache given its new size (5) |
||
| // verifiers | ||
| if descriptor.Verifier != nil { | ||
| entry := &VerifierEntry{Verifier: descriptor.Verifier} | ||
| if p.Logger.IsEnabledFor(zapcore.DebugLevel) { | ||
| entry.DebugStack = debug.Stack() | ||
| } | ||
| p.verifiers.Add(id, entry) | ||
| if setAlias { | ||
| p.verifiers.Add(aliasID, entry) | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like this function is not really needed anymore. I would start an investigation on the callers of this function. Maybe we can save further cycles.