Skip to content

Commit

Permalink
utilccl: cache license decoding
Browse files Browse the repository at this point in the history
Previously, the `utilccl` package would decode the license from the the
base64-encoded Protobuf representation in settings every time it was
needed, which was sufficient for its uses. However, recently there's
been a need to check whether enterprise features are enabled in hot
paths (e.g. with follower reads as seen in #62447), making the decoding
cost too great.

This patch adds `cluster.Settings.Cache` as a shared cache, and uses it
to cache decoded licenses with a private key type.

Release note: None
  • Loading branch information
erikgrinaker committed Mar 31, 2021
1 parent 0f424ea commit a7c1f37
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 28 deletions.
60 changes: 35 additions & 25 deletions pkg/ccl/utilccl/license_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ const (
var errEnterpriseRequired = pgerror.New(pgcode.CCLValidLicenseRequired,
"a valid enterprise license is required")

// licenseCacheKey is used to cache licenses in cluster.Settings.Cache,
// keeping the entries private.
type licenseCacheKey string

// TestingEnableEnterprise allows overriding the license check in tests.
func TestingEnableEnterprise() func() {
before := atomic.LoadInt32(&testingEnterprise)
Expand Down Expand Up @@ -112,19 +116,12 @@ func init() {
func TimeToEnterpriseLicenseExpiry(
ctx context.Context, st *cluster.Settings, asOf time.Time,
) (time.Duration, error) {
var lic *licenseccl.License
// FIXME(tschottdorf): see whether it makes sense to cache the decoded
// license.
if str := enterpriseLicense.Get(&st.SV); str != "" {
var err error
if lic, err = decode(str); err != nil {
return 0, err
}
} else {
return 0, nil
license, err := getLicense(st)
if err != nil || license == nil {
return 0, err
}

expiration := timeutil.Unix(lic.ValidUntilUnixSec, 0)
expiration := timeutil.Unix(license.ValidUntilUnixSec, 0)
return expiration.Sub(asOf), nil
}

Expand All @@ -134,28 +131,41 @@ func checkEnterpriseEnabledAt(
if atomic.LoadInt32(&testingEnterprise) == testingEnterpriseEnabled {
return nil
}
var lic *licenseccl.License
// FIXME(tschottdorf): see whether it makes sense to cache the decoded
// license.
if str := enterpriseLicense.Get(&st.SV); str != "" {
var err error
if lic, err = decode(str); err != nil {
return err
}
license, err := getLicense(st)
if err != nil {
return err
}
return check(lic, at, cluster, org, feature, withDetails)
return check(license, at, cluster, org, feature, withDetails)
}

func getLicenseType(st *cluster.Settings) (string, error) {
// getLicense fetches the license from the given settings, using Settings.Cache
// to cache the decoded license (if any). The returned license must not be
// modified by the caller.
func getLicense(st *cluster.Settings) (*licenseccl.License, error) {
str := enterpriseLicense.Get(&st.SV)
if str == "" {
return "None", nil
return nil, nil
}
lic, err := decode(str)
cacheKey := licenseCacheKey(str)
if cachedLicense, ok := st.Cache.Load(cacheKey); ok {
return cachedLicense.(*licenseccl.License), nil
}
license, err := decode(str)
if err != nil {
return nil, err
}
st.Cache.Store(cacheKey, license)
return license, nil
}

func getLicenseType(st *cluster.Settings) (string, error) {
license, err := getLicense(st)
if err != nil {
return "", err
} else if license == nil {
return "None", nil
}
return lic.Type.String(), nil
return license.Type.String(), nil
}

// decode attempts to read a base64 encoded License.
Expand All @@ -164,7 +174,7 @@ func decode(s string) (*licenseccl.License, error) {
if err != nil {
return nil, pgerror.WithCandidateCode(err, pgcode.Syntax)
}
return lic, err
return lic, nil
}

// check returns an error if the license is empty or not currently valid. If
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/utilccl/license_check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestSettingAndCheckingLicense(t *testing.T) {
if err := updater.Set("enterprise.license", tc.lic, "s"); err != nil {
t.Fatal(err)
}
err := checkEnterpriseEnabledAt(st, tc.checkTime, tc.checkCluster, "", "")
err := checkEnterpriseEnabledAt(st, tc.checkTime, tc.checkCluster, "", "", true)
if !testutils.IsError(err, tc.err) {
l, _ := decode(tc.lic)
t.Fatalf("%d: lic %v, update by %T, checked by %s at %s, got %q", i, l, updater, tc.checkCluster, tc.checkTime, err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/utilccl/license_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func TestLicense(t *testing.T) {
}
}
if err := check(
lic, tc.checkTime, tc.checkCluster, tc.checkOrg, "",
lic, tc.checkTime, tc.checkCluster, tc.checkOrg, "", true,
); !testutils.IsError(err, tc.err) {
t.Fatalf("%d: lic for %s to %s, checked by %s at %s.\n got %q", i,
tc.grantedTo, tc.expiration, tc.checkCluster, tc.checkTime, err)
Expand All @@ -108,7 +108,7 @@ func TestExpiredLicenseLanguage(t *testing.T) {
Type: licenseccl.License_Evaluation,
ValidUntilUnixSec: 1,
}
err := check(lic, timeutil.Now(), uuid.MakeV4(), "", "RESTORE")
err := check(lic, timeutil.Now(), uuid.MakeV4(), "", "RESTORE", true)
expected := "Use of RESTORE requires an enterprise license. Your evaluation license expired on " +
"January 1, 1970. If you're interested in getting a new license, please contact " +
"[email protected] and we can help you out."
Expand Down
5 changes: 5 additions & 0 deletions pkg/settings/cluster/cluster_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ package cluster

import (
"context"
"sync"
"sync/atomic"

"github.com/cockroachdb/cockroach/pkg/clusterversion"
Expand Down Expand Up @@ -51,6 +52,10 @@ type Settings struct {
// Setting the active cluster version has a very specific, intended usage
// pattern. Look towards the interface itself for more commentary.
Version clusterversion.Handle

// Cache can be used for arbitrary caching, e.g. to cache decoded
// enterprises licenses for utilccl.CheckEnterpriseEnabled().
Cache sync.Map
}

// TelemetryOptOut is a place for controlling whether to opt out of telemetry or not.
Expand Down

0 comments on commit a7c1f37

Please sign in to comment.