Skip to content

feat(cancel): Graceful cancellation to GetChangedTargets #58

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.

Sign up for GitHub

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

Merged
sbalabanov merged 1 commit into from
Jun 9, 2026
+48 −11
Merged

feat(cancel): Graceful cancellation to GetChangedTargets #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 40 additions & 3 deletions controller/getchangedtargets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ type job struct {
cancel context.CancelFunc
}

// GetChangedTargets returns the changed targets between two revisions.
// GetChangedTargets returns the changed targets between two revisions. If the
// client disconnects, the stream's context is cancelled and the function
// returns with context.Canceled.
func (c *controller) GetChangedTargets(request *pb.GetChangedTargetsRequest, stream pb.TangoServiceGetChangedTargetsYARPCServer) (retErr error) {
scope := c.scope.SubScope("get_changed_targets")
scope.Counter("calls").Inc(1)
Expand Down Expand Up @@ -239,7 +241,7 @@ func (c *controller) GetChangedTargets(request *pb.GetChangedTargetsRequest, str
jobs[1].graphStreamChunks = nil

compareStart := time.Now()
changedTargetsResponses, err := c.compareTargetGraphs(logger, firstGraph, secondGraph, maxDist, request.GetOutputConfig().GetComputeDistances())
changedTargetsResponses, err := c.compareTargetGraphs(ctx, logger, firstGraph, secondGraph, maxDist, request.GetOutputConfig().GetComputeDistances())
// Allow GC of raw graph data while the caching goroutine runs.
firstGraph = nil
secondGraph = nil
Expand Down Expand Up @@ -294,27 +296,50 @@ func (c *controller) GetChangedTargets(request *pb.GetChangedTargetsRequest, str
// requested or when filtering by distance is active. Output IDs are
// re-mapped into a canonical per-call namespace so the response metadata
// only carries the names actually referenced.
func (c *controller) compareTargetGraphs(logger *zap.Logger, firstGraph, secondGraph []*pb.GetTargetGraphResponse, maxDist int32, outputDistances bool) ([]*pb.GetChangedTargetsResponse, error) {
func (c *controller) compareTargetGraphs(ctx context.Context, logger *zap.Logger, firstGraph, secondGraph []*pb.GetTargetGraphResponse, maxDist int32, outputDistances bool) ([]*pb.GetChangedTargetsResponse, error) {
start := time.Now()
scope := c.scope.SubScope("compare_target_graphs")
logger.Info("compareTargetGraphs: Computing differences between target graphs")

// 1) Extract targets and metadata; index by canonical names
indexStart := time.Now()
firstTargetsByID, firstMetadata := getTargetsAndMetadata(firstGraph)

if ctx.Err() != nil {
return nil, ctx.Err()
}

secondTargetsByID, secondMetadata := getTargetsAndMetadata(secondGraph)

if ctx.Err() != nil {
return nil, ctx.Err()
}

// Release raw chunk slices — individual target protos are now held by the ID maps.
firstGraph = nil
secondGraph = nil
firstByName := buildNameIndex(firstTargetsByID, firstMetadata)
firstTargetsByID = nil // all pointers are now in firstByName; drop the duplicate map

if ctx.Err() != nil {
return nil, ctx.Err()
}

secondByName := buildNameIndex(secondTargetsByID, secondMetadata)
secondTargetsByID = nil
indexDuration := time.Since(indexStart)
scope.Timer("index_duration").Record(indexDuration)

if ctx.Err() != nil {
return nil, ctx.Err()
}

sourceFileRuleTypeID := detectSourceFileID(secondMetadata)

if ctx.Err() != nil {
return nil, ctx.Err()
}

changedByName := make(map[string]*pb.ChangedTarget)
changedSourceFileTargets := make(map[string]struct{})

Expand Down Expand Up @@ -399,6 +424,10 @@ func (c *controller) compareTargetGraphs(logger *zap.Logger, firstGraph, secondG
diffScanDuration := time.Since(diffScanStart)
scope.Timer("diff_scan_duration").Record(diffScanDuration)

if ctx.Err() != nil {
return nil, ctx.Err()
}

// Iterate over the changed targets and check if any of them are DIRECT changes.
classifyStart := time.Now()
for name, ct := range changedByName {
Expand Down Expand Up @@ -436,6 +465,10 @@ func (c *controller) compareTargetGraphs(logger *zap.Logger, firstGraph, secondG
classifyDuration := time.Since(classifyStart)
scope.Timer("classify_duration").Record(classifyDuration)

if ctx.Err() != nil {
return nil, ctx.Err()
}

// Compute BFS distances when filtering is active or the client requested distance output.
if maxDist >= 0 || outputDistances {
distancesStart := time.Now()
Expand All @@ -444,6 +477,10 @@ func (c *controller) compareTargetGraphs(logger *zap.Logger, firstGraph, secondG
scope.Timer("distances_duration").Record(distancesDuration)
}

if ctx.Err() != nil {
return nil, ctx.Err()
}

// Collect changed targets.
changed := make([]*pb.ChangedTarget, 0, len(changedByName))
for _, ct := range changedByName {
Expand Down
16 changes: 8 additions & 8 deletions controller/getchangedtargets_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func TestCompareTargetGraphs(t *testing.T) {
},
}

response, err := c.compareTargetGraphs(zap.NewNop(), []*pb.GetTargetGraphResponse{firstGraph}, []*pb.GetTargetGraphResponse{secondGraph}, -1, false)
response, err := c.compareTargetGraphs(context.Background(), zap.NewNop(), []*pb.GetTargetGraphResponse{firstGraph}, []*pb.GetTargetGraphResponse{secondGraph}, -1, false)
require.NoError(t, err)
require.NotNil(t, response)
}
Expand Down Expand Up @@ -436,7 +436,7 @@ func TestCompareTargetGraphs_NewTarget_CanonicalIDs(t *testing.T) {
},
},
}
res, err := c.compareTargetGraphs(zap.NewNop(), first, second, -1, false)
res, err := c.compareTargetGraphs(context.Background(), zap.NewNop(), first, second, -1, false)
require.NoError(t, err)
require.Len(t, res, 2)
cs := res[0].GetChangedTargets()
Expand Down Expand Up @@ -508,7 +508,7 @@ func TestCompareTargetGraphs_SourceFileDirectAndPropagation(t *testing.T) {
},
},
}
res, err := c.compareTargetGraphs(zap.NewNop(), first, second, -1, false)
res, err := c.compareTargetGraphs(context.Background(), zap.NewNop(), first, second, -1, false)
require.NoError(t, err)
cs := res[0].GetChangedTargets()
require.NotNil(t, cs)
Expand Down Expand Up @@ -576,7 +576,7 @@ func TestCompareTargetGraphs_IndirectWhenNoSourceDep(t *testing.T) {
},
},
}
res, err := c.compareTargetGraphs(zap.NewNop(), first, second, -1, false)
res, err := c.compareTargetGraphs(context.Background(), zap.NewNop(), first, second, -1, false)
require.NoError(t, err)
cs := res[0].GetChangedTargets()
require.NotNil(t, cs)
Expand Down Expand Up @@ -641,7 +641,7 @@ func TestCompareTargetGraphs_DirectWhenDependenciesChanged(t *testing.T) {
},
},
}
res, err := c.compareTargetGraphs(zap.NewNop(), first, second, -1, false)
res, err := c.compareTargetGraphs(context.Background(), zap.NewNop(), first, second, -1, false)
require.NoError(t, err)
cs := res[0].GetChangedTargets()
require.NotNil(t, cs)
Expand Down Expand Up @@ -722,7 +722,7 @@ func TestCompareTargetGraphs_DirectWhenAttributesChanged(t *testing.T) {
},
},
}
res, err := c.compareTargetGraphs(zap.NewNop(), first, second, -1, false)
res, err := c.compareTargetGraphs(context.Background(), zap.NewNop(), first, second, -1, false)
require.NoError(t, err)
cs := res[0].GetChangedTargets()
require.NotNil(t, cs)
Expand Down Expand Up @@ -802,7 +802,7 @@ func TestCompareTargetGraphs_DirectWhenNewAttributeAdded(t *testing.T) {
},
},
}
res, err := c.compareTargetGraphs(zap.NewNop(), first, second, -1, false)
res, err := c.compareTargetGraphs(context.Background(), zap.NewNop(), first, second, -1, false)
require.NoError(t, err)
cs := res[0].GetChangedTargets()
require.NotNil(t, cs)
Expand Down Expand Up @@ -1101,7 +1101,7 @@ func TestCompareTargetGraphs_IndirectWhenOnlyHashChanged(t *testing.T) {
},
},
}
res, err := c.compareTargetGraphs(zap.NewNop(), first, second, -1, false)
res, err := c.compareTargetGraphs(context.Background(), zap.NewNop(), first, second, -1, false)
require.NoError(t, err)
cs := res[0].GetChangedTargets()
require.NotNil(t, cs)
Expand Down
Loading