Skip to content

Commit

Permalink
Merge pull request keybase#8460 from keybase/miles/CORE-6032-teamlist…
Browse files Browse the repository at this point in the history 
…-perf-2

TeamList with All is fast
  • Loading branch information
@mlsteele
mlsteele authored Sep 18, 2017
2 parents 445038a + bc3ea05 commit 4b70c8e
Show file tree
Hide file tree
Showing 4 changed files with 189 additions and 32 deletions.
53 changes: 53 additions & 0 deletions go/libkb/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"regexp"
"runtime"
"strings"
"sync"
"time"
"unicode"

Expand Down Expand Up @@ -506,6 +507,10 @@ func (g *GlobalContext) CTraceTimed(ctx context.Context, msg string, f func() er
return CTraceTimed(ctx, g.Log, msg, f, g.Clock())
}

func (g *GlobalContext) CTimeTracer(ctx context.Context, label string) *TimeTracer {
return NewTimeTracer(ctx, g.Log, g.Clock(), label)
}

func (g *GlobalContext) ExitTraceOK(msg string, f func() bool) func() {
return func() { g.Log.Debug("| %s -> %v", msg, f()) }
}
Expand Down Expand Up @@ -675,3 +680,51 @@ func CITimeMultiplier(g Contextifier) time.Duration {
}
return time.Duration(1)
}

type TimeTracer struct {
sync.Mutex
ctx context.Context
log logger.Logger
clock clockwork.Clock
label string
stage string
staged bool // whether any stages were used
start time.Time // when the tracer started
prev time.Time // when the active stage started
}

func NewTimeTracer(ctx context.Context, log logger.Logger, clock clockwork.Clock, label string) *TimeTracer {
now := clock.Now()
return &TimeTracer{
ctx: ctx,
log: log,
clock: clock,
label: label,
stage: "init",
staged: false,
start: now,
prev: now,
}
}

func (t *TimeTracer) finishStage() {
t.log.CDebugf(t.ctx, "| %s:%s [time=%s]", t.label, t.stage, t.clock.Since(t.prev))
}

func (t *TimeTracer) Stage(format string, args ...interface{}) {
t.Lock()
defer t.Unlock()
t.finishStage()
t.stage = fmt.Sprintf(format, args...)
t.prev = t.clock.Now()
t.staged = true
}

func (t *TimeTracer) Finish() {
t.Lock()
defer t.Unlock()
if t.staged {
t.finishStage()
}
t.log.CDebugf(t.ctx, "- %s [time=%s]", t.label, t.clock.Since(t.start))
}
19 changes: 15 additions & 4 deletions go/teams/list.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ func getTeamsListFromServer(ctx context.Context, g *libkb.GlobalContext, uid key
}

func List(ctx context.Context, g *libkb.GlobalContext, arg keybase1.TeamListArg) (*keybase1.AnnotatedTeamList, error) {
tracer := g.CTimeTracer(ctx, "TeamList")
defer tracer.Finish()

var uid keybase1.UID
if arg.UserAssertion != "" {
res := g.Resolver.ResolveFullExpression(ctx, arg.UserAssertion)
Expand All @@ -54,12 +57,14 @@ func List(ctx context.Context, g *libkb.GlobalContext, arg keybase1.TeamListArg)

meUID := g.ActiveDevice.UID()

tracer.Stage("server")
teams, err := getTeamsListFromServer(ctx, g, uid, arg.All)
if err != nil {
return nil, err
}

teamNames := make(map[string]bool)
tracer.Stage("loop1")
teamList := make(map[string]keybase1.TeamID)
upakLoader := g.GetUPAKLoader()
var annotatedTeams []keybase1.AnnotatedMemberInfo
administeredTeams := make(map[string]bool)
Expand All @@ -69,7 +74,7 @@ func List(ctx context.Context, g *libkb.GlobalContext, arg keybase1.TeamListArg)
continue
}

teamNames[memberInfo.FqName] = true
teamList[memberInfo.FqName] = memberInfo.TeamID
if memberInfo.UserID == meUID && (memberInfo.Role.IsAdminOrAbove() || (memberInfo.Implicit != nil && memberInfo.Implicit.Role.IsAdminOrAbove())) {
administeredTeams[memberInfo.FqName] = true
}
Expand All @@ -95,18 +100,24 @@ func List(ctx context.Context, g *libkb.GlobalContext, arg keybase1.TeamListArg)
})
}

tracer.Stage("loop2")
annotatedInvites := make(map[keybase1.TeamInviteID]keybase1.AnnotatedTeamInvite)
for teamName := range teamNames {
for teamName, teamID := range teamList {
_, ok := administeredTeams[teamName]
if ok {
t, err := Load(ctx, g, keybase1.LoadTeamArg{
Name: teamName,
ID: teamID,
NeedAdmin: true,
})
if err != nil {
g.Log.Warning("Error while getting team (%s): %v", teamName, err)
continue
}
if t.Name().String() != teamName {
// This could trigger if we have cached an old name and have not gotten updates.
g.Log.Warning("Error while getting team (%s): %v", teamName, fmt.Errorf("team name mismatch"))
continue
}
teamAnnotatedInvites, err := AnnotateInvites(ctx, g, t.chain().inner.ActiveInvites, teamName)
if err != nil {
return nil, err
Expand Down
138 changes: 113 additions & 25 deletions go/teams/loader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ func (l *TeamLoader) load1(ctx context.Context, me keybase1.UserVersion, lArg ke
mungedWantMembers = nil
}

var ret *keybase1.TeamData
ret, err = l.load2(ctx, load2ArgT{
ret, err := l.load2(ctx, load2ArgT{
teamID: teamID,

needAdmin: lArg.NeedAdmin,
Expand All @@ -129,7 +128,7 @@ func (l *TeamLoader) load1(ctx context.Context, me keybase1.UserVersion, lArg ke
// Sanity check that secretless teams never escape.
// They are meant only to be returned by recursively called load2's
// and used internally by ImplicitAdmins.
if ret.Secretless {
if ret.team.Secretless {
return nil, fmt.Errorf("team loader fault: got secretless team")
}

Expand All @@ -138,12 +137,12 @@ func (l *TeamLoader) load1(ctx context.Context, me keybase1.UserVersion, lArg ke
// because the cache is keyed by ID.
if teamName != nil {
// (TODO: this won't work for renamed level 3 teams or above. There's work on this in miles/teamloader-names)
if !teamName.Eq(ret.Name) {
return nil, fmt.Errorf("team name mismatch: %v != %v", ret.Name, teamName.String())
if !teamName.Eq(ret.team.Name) {
return nil, fmt.Errorf("team name mismatch: %v != %v", ret.team.Name, teamName.String())
}
}

return ret, nil
return &ret.team, nil
}

func (l *TeamLoader) checkArg(ctx context.Context, lArg keybase1.LoadTeamArg) error {
Expand All @@ -166,6 +165,8 @@ func (l *TeamLoader) checkArg(ctx context.Context, lArg keybase1.LoadTeamArg) er
type load2ArgT struct {
teamID keybase1.TeamID

reason string // optional tag for debugging why this load is happening

needAdmin bool
needKeyGeneration keybase1.PerTeamKeyGeneration
// wantMembers here is different from wantMembers on LoadTeamArg:
Expand All @@ -187,17 +188,27 @@ type load2ArgT struct {
me keybase1.UserVersion
}

type load2ResT struct {
team keybase1.TeamData
didRepoll bool
}

// Load2 does the rest of the work loading a team.
// It is `playchain` described in the pseudocode in teamplayer.txt
func (l *TeamLoader) load2(ctx context.Context, arg load2ArgT) (ret *keybase1.TeamData, err error) {
ctx = libkb.WithLogTag(ctx, "LT")
defer l.G().CTraceTimed(ctx, fmt.Sprintf("TeamLoader#load2(%v)", arg.teamID), func() error { return err })()
func (l *TeamLoader) load2(ctx context.Context, arg load2ArgT) (ret *load2ResT, err error) {
ctx = libkb.WithLogTag(ctx, "LT") // Load Team
traceLabel := fmt.Sprintf("TeamLoader#load2(%v)", arg.teamID)
if len(arg.reason) > 0 {
traceLabel = traceLabel + " '" + arg.reason + "'"
}
defer l.G().CTraceTimed(ctx, traceLabel, func() error { return err })()
ret, err = l.load2Inner(ctx, arg)
return ret, err
}

func (l *TeamLoader) load2Inner(ctx context.Context, arg load2ArgT) (*keybase1.TeamData, error) {
func (l *TeamLoader) load2Inner(ctx context.Context, arg load2ArgT) (*load2ResT, error) {
var err error
var didRepoll bool

// Single-flight lock by team ID.
lock := l.locktab.AcquireOnName(ctx, l.G(), arg.teamID.String())
Expand Down Expand Up @@ -249,6 +260,7 @@ func (l *TeamLoader) load2Inner(ctx context.Context, arg load2ArgT) (*keybase1.T
if err != nil {
return nil, err
}
didRepoll = true
} else {
lastSeqno = ret.Chain.LastSeqno
lastLinkID = ret.Chain.LastLinkID
Expand Down Expand Up @@ -409,7 +421,10 @@ func (l *TeamLoader) load2Inner(ctx context.Context, arg load2ArgT) (*keybase1.T
return nil, err
}

return ret, nil
return &load2ResT{
team: *ret,
didRepoll: didRepoll,
}, nil
}

// Decide whether to repoll merkle based on load arg.
Expand Down Expand Up @@ -525,23 +540,16 @@ func (l *TeamLoader) satisfiesNeedAdmin(ctx context.Context, me keybase1.UserVer
l.G().Log.CDebugf(ctx, "TeamLoader error getting my role: %v", err)
return false
}
if !(role == keybase1.TeamRole_OWNER || role == keybase1.TeamRole_ADMIN) {
if !role.IsAdminOrAbove() {
if !state.IsSubteam() {
return false
}
implicitAdmins, err := l.implicitAdminsAncestor(ctx, state.GetID(), state.GetParentID())
yes, err := l.isImplicitAdminOf(ctx, state.GetID(), state.GetParentID(), me, me)
if err != nil {
l.G().Log.CDebugf(ctx, "TeamLoader error getting implicit admins: %s", err)
l.G().Log.CDebugf(ctx, "TeamLoader error getting checking implicit admin: %s", err)
return false
}
found := false
for _, ia := range implicitAdmins {
if ia == me {
found = true
break
}
}
if !found {
if !yes {
return false
}
}
Expand All @@ -551,6 +559,83 @@ func (l *TeamLoader) satisfiesNeedAdmin(ctx context.Context, me keybase1.UserVer
return true
}

// Check whether a user is an implicit admin of a team.
func (l *TeamLoader) isImplicitAdminOf(ctx context.Context, teamID keybase1.TeamID, ancestorID *keybase1.TeamID,
me keybase1.UserVersion, uv keybase1.UserVersion) (bool, error) {

// IDs of ancestors that were not freshly polled.
// Check them again with forceRepoll if the affirmitive is not found cached.
checkAgain := make(map[keybase1.TeamID]bool)

check1 := func(chain *TeamSigChainState) bool {
role, err := chain.GetUserRole(uv)
if err != nil {
return false
}
return role.IsAdminOrAbove()
}

i := 0
for {
i++
if i >= 100 {
// Break in case there's a bug in this loop.
return false, fmt.Errorf("stuck in a loop while checking for implicit admin: %v", ancestorID)
}

// Use load2 so that we can use subteam-reader and get secretless teams.
ancestor, err := l.load2(ctx, load2ArgT{
teamID: *ancestorID,
reason: "isImplicitAdminOf-1",
me: me,
readSubteamID: &teamID,
})
if err != nil {
return false, err
}
// Be wary, `ancestor` could be, and is likely, a secretless team.
// Do not let it out of sight.
ancestorChain := TeamSigChainState{inner: ancestor.team.Chain}

if !ancestor.didRepoll {
checkAgain[ancestorChain.GetID()] = true
}

if check1(&ancestorChain) {
return true, nil
}

if !ancestorChain.IsSubteam() {
break
}
// Get the next level up.
ancestorID = ancestorChain.GetParentID()
}

// The answer was not found to be yes in the cache.
// Try again with the teams that were not polled as they might have unseen updates.
for ancestorID, _ := range checkAgain {
ancestor, err := l.load2(ctx, load2ArgT{
teamID: ancestorID,
reason: "isImplicitAdminOf-again",
me: me,
forceRepoll: true, // Get the latest info.
readSubteamID: &teamID,
})
if err != nil {
return false, err
}
// Be wary, `ancestor` could be, and is likely, a secretless team.
// Do not let it out of sight.
ancestorChain := TeamSigChainState{inner: ancestor.team.Chain}
if check1(&ancestorChain) {
return true, nil
}
}

return false, nil
}

// Whether the snapshot has loaded at least up to the key generation.
func (l *TeamLoader) satisfiesNeedKeyGeneration(ctx context.Context, needKeyGeneration keybase1.PerTeamKeyGeneration, state *keybase1.TeamData) error {
if needKeyGeneration == 0 {
Expand Down Expand Up @@ -731,6 +816,7 @@ func (l *TeamLoader) implicitAdminsAncestor(ctx context.Context, teamID keybase1
// Use load2 so that we can use subteam-reader and get secretless teams.
ancestor, err := l.load2(ctx, load2ArgT{
teamID: *ancestorID,
reason: "implicitAdminsAncestor",
me: me,
forceRepoll: true, // Get the latest info.
readSubteamID: &teamID,
Expand All @@ -740,7 +826,7 @@ func (l *TeamLoader) implicitAdminsAncestor(ctx context.Context, teamID keybase1
}
// Be wary, `ancestor` could be, and is likely, a secretless team.
// Do not let it out of sight.
ancestorChain := TeamSigChainState{inner: ancestor.Chain}
ancestorChain := TeamSigChainState{inner: ancestor.team.Chain}

// Gather the admins.
adminRoles := []keybase1.TeamRole{keybase1.TeamRole_OWNER, keybase1.TeamRole_ADMIN}
Expand Down Expand Up @@ -790,8 +876,9 @@ func (l *TeamLoader) NotifyTeamRename(ctx context.Context, id keybase1.TeamID, n

loopID := &id
for loopID != nil {
team, err := l.load2(ctx, load2ArgT{
load2Res, err := l.load2(ctx, load2ArgT{
teamID: *loopID,
reason: "NotifyTeamRename-force",
forceRepoll: true,
readSubteamID: &id,
me: me,
Expand All @@ -800,7 +887,7 @@ func (l *TeamLoader) NotifyTeamRename(ctx context.Context, id keybase1.TeamID, n
return err
}
ancestorIDs = append(ancestorIDs, *loopID)
chain := TeamSigChainState{inner: team.Chain}
chain := TeamSigChainState{inner: load2Res.team.Chain}
if chain.IsSubteam() {
loopID = chain.GetParentID()
} else {
Expand All @@ -814,6 +901,7 @@ func (l *TeamLoader) NotifyTeamRename(ctx context.Context, id keybase1.TeamID, n
for _, loopID := range ancestorIDs {
_, err := l.load2(ctx, load2ArgT{
teamID: loopID,
reason: "NotifyTeamRename-quick",
readSubteamID: &id,
me: me,
})
Expand Down
Loading

0 comments on commit 4b70c8e

Please sign in to comment.