Skip to content

algo.localClusteringCoefficient is abortable on OLTP and not on CSR, and nothing stops the next procedure from shipping without a checkpoint either #6318

Description

@lvca

Two leftovers from #6302 (PR #6306), stated together because they are the same question - how does the abortability guarantee hold once the PR that established it is merged - approached from opposite ends.

1. The one procedure the sweep could not cover

#6302 swept all 68 algo.* procedures and gave a WorkGuard checkpoint to the 22 whose dominant loop is superlinear in the graph. algo.localClusteringCoefficient got one on its OLTP path only, and the code says so rather than hiding it:

// AlgoLocalClusteringCoefficient.executeWithOLTP
// Only the OLTP path is covered here. The CSR path hands the whole computation to
// GraphAlgorithms#localClusteringCoefficient, which counts triangles across a thread pool, and the
// WorkCheckpoint hook #6264 introduced is specified to be called between iterations on the calling
// thread rather than from inside a parallel chunk [...]

So the same query is abortable without a Graph Analytical View and unabortable with one - and the CSR path is the one taken on the large graphs where it matters. The cost is O(m x sqrt(m)) triangle intersections with nothing but the graph sizing it.

WorkCheckpoint's contract is the obstacle, and it is a deliberate one: "called from the calling thread between two iterations, never from inside a parallel chunk, so an implementation does not have to be thread-safe and the exception it throws propagates straight to the caller". lccBuildAndIntersect partitions the node range across a Thread[] and has no between-iterations point to hook.

Two ways out:

  • Chunk the parallel phase. Partition into k batches instead of one-per-thread, join between batches, and check there. Costs k barriers; abort latency becomes one batch. Keeps WorkCheckpoint single-threaded as specified.
  • Widen the contract to a thread-safe volatile flag each worker polls, with the exception raised by the joining thread. Lower latency, but every kernel then has to be audited for it, and the current contract exists precisely to avoid that.

The first is the smaller change and fits the existing hook. Worth doing for the whole GraphAlgorithms parallel family rather than for LCC alone, since pageRank and labelPropagation already take a WorkCheckpoint and would gain the same intra-iteration granularity from it.

2. The convention has no backstop

Both review rounds on PR #6306 raised this independently, and it is the reason the bug class existed in the first place. newWorkGuard(context) plus a hand-placed guard.check() is a convention copied file to file across 22 classes. Issue6302AlgoGraphDrivenWorkGuardTest's parameterised table is the only thing that would notice a new procedure shipping without one - and only if somebody remembers to add a row for it.

A base-class hook was considered and is the wrong answer: the placement is a per-loop judgement (unthrottled check() where one iteration is already O(V), checkPeriodically on a running counter where the unbounded thing is the product of two loops), so a hook would standardise the part that is already easy and leave the part that is hard. #6264 reached the same conclusion over three review rounds.

What would actually catch it is a check over the package, roughly: a loop whose bound is nodeCount, edgeCount or an adjacency length, inside execute(), with no guard reference anywhere in its body. As a test it can be a source scan asserting against a known allow-list; as tooling it is an ArchUnit-style rule or a Codacy pattern. Either way it turns "someone has to remember" into "the build says so", which is the difference between the 22 staying guarded and drifting.

The same check would have found the #6295 instances directly: every one of them was a loop over nodeCount in a class that already had a guard, just not in that phase.

Related

#6302, #6295, #6264, #6216, PR #6306.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions