Skip to content

Commit

Permalink
speed up NeedAdmin for subteams
Browse files Browse the repository at this point in the history
  • Loading branch information
@mlsteele
mlsteele committed Sep 18, 2017
1 parent cb40493 commit 72f2e6e
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 25 deletions.
123 changes: 101 additions & 22 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 Down Expand Up @@ -189,9 +188,14 @@ 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) {
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 {
Expand All @@ -202,8 +206,9 @@ func (l *TeamLoader) load2(ctx context.Context, arg load2ArgT) (ret *keybase1.Te
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 @@ -255,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 @@ -415,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 @@ -535,19 +544,12 @@ func (l *TeamLoader) satisfiesNeedAdmin(ctx context.Context, me keybase1.UserVer
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 @@ -557,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 @@ -747,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 @@ -797,7 +876,7 @@ 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,
Expand All @@ -808,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 Down
7 changes: 4 additions & 3 deletions go/teams/loader2.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,11 @@ func (l *TeamLoader) walkUpToAdmin(
if target.Eq(*parent) {
arg.needSeqnos = []keybase1.Seqno{admin.Seqno}
}
team, err = l.load2(ctx, arg)
load2Res, err := l.load2(ctx, arg)
if err != nil {
return nil, err
}
team = &load2Res.team
}
if team == nil {
return nil, fmt.Errorf("teamloader fault: nil team after admin walk")
Expand Down Expand Up @@ -476,7 +477,7 @@ func (l *TeamLoader) checkParentChildOperations(ctx context.Context,
}

for _, pco := range parentChildOperations {
parentChain := TeamSigChainState{inner: parent.Chain}
parentChain := TeamSigChainState{inner: parent.team.Chain}
err = l.checkOneParentChildOperation(ctx, pco, loadingTeamID, &parentChain)
if err != nil {
return err
Expand Down Expand Up @@ -802,7 +803,7 @@ func (l *TeamLoader) calculateName(ctx context.Context,
// Swap out the parent name as the base of this name.
// Check that the root ancestor name and depth still match the subteam chain.

newName, err = parent.Name.Append(string(chain.LatestLastNamePart()))
newName, err = parent.team.Name.Append(string(chain.LatestLastNamePart()))
if err != nil {
return newName, fmt.Errorf("invalid new subteam name: %v", err)
}
Expand Down

0 comments on commit 72f2e6e

Please sign in to comment.