Skip to content

Thirteen more algo.* procedures have the unguarded iteration knob #6216 fixed in two (follow-up to #6216) #6264

Description

@lvca

Follow-up to #6216 (PR #6222). That issue named three procedures - algo.node2vec, algo.maxKCut, algo.influenceMaximization - and they are fixed. The same two defects are present, unchanged, in thirteen more procedures that were never in scope.

The two defects

#6216 established that an iteration-shaped knob needs two things:

  1. A domain minimum, rejected by name. Below its minimum such a knob does not mean "a smaller run", it means an answer the algorithm cannot produce - and the value is absorbed in silence. algo.maxKCut(2, {restarts: 0}) used to return every node in community 0 with a cut weight of -1.0, reported as success.
  2. A checkpoint inside the loop it drives, so a large value is abortable via thread interrupt and arcadedb.command.timeout rather than forbidden by a guessed cap.

The thirteen

Every one of these extracts its knob with a plain extractInt(n, "maxIterations") - no minimum - and contains zero references to WorkGuard:

procedure knob default
AlgoPageRank maxIterations 20
AlgoPersonalizedPageRank maxIterations 20
AlgoArticleRank maxIterations 20
AlgoEigenvectorCentrality maxIterations 100
AlgoHITS maxIterations 20
AlgoKatz maxIterations 10
AlgoLouvain maxIterations 10
AlgoLeiden maxIterations 10
AlgoLabelPropagation maxIterations 10
AlgoSLPA iterations 20
AlgoSimRank maxIterations 5
AlgoFastRP iterations 4
AlgoHashGNN iterations 4

Verified against the merged tree: grep -ln "maxIterations|\"iterations\"" *.java returns 15 files, and only AlgoNode2Vec and AlgoMaxKCut - the two #6216 touched - contain any guard. call.

So CALL algo.pageRank({maxIterations: 0}) returns the uniform initial rank vector as though it were a PageRank result, and CALL algo.pageRank({maxIterations: 2000000000}) spins with no way to interrupt it or to have arcadedb.command.timeout observed. Both are what #6216 fixed for algo.maxKCut.

The silent-wrong-answer half is the more serious one: an unconverged or un-iterated centrality is not obviously wrong to a caller, unlike an exception.

Why it was out of scope then and should be in scope now

#6216 was scoped to the knobs its parent review had actually named, and #6222 deliberately did not widen it - a validation fix that also rewrites thirteen unrelated procedures is hard to review, and the mechanism it needed (extractInt(value, name, minimum), WorkGuard, checkPeriodically) did not exist yet. It exists now and is in AbstractAlgoProcedure, so this is mechanical application rather than design.

Notes for whoever picks this up

Three things #6222 learned the hard way, all of which apply here:

  • A checkpoint goes inside the loop whose trip count the knob controls, not around it. fix(#6216): a graph-algorithm knob is bounded by the resource it spends, not by a guessed cap #6222 spent three review cycles on this: a checkpoint between walks does not bound a single walk that is O(walkLength squared). For an iterative centrality the outer iteration loop is usually the right place and the per-node scan within an iteration, since the latter is O(nodeCount).
  • Throttle with checkPeriodically only where one iteration can be cheaper than the check itself. Where an iteration already allocates or scans the graph, the unthrottled check() gives better abort latency at no measurable cost.
  • A latency regression test cannot assert only "it throws". If an outer checkpoint still exists, the unguarded version does throw - just late. fix(#6216): a graph-algorithm knob is bounded by the resource it spends, not by a guessed cap #6222 had one such test silently pass against broken code before it was re-measured. Assert elapsed time, and set the bound from measuring both sides.

Some of these procedures also have a tolerance convergence check that may exit before maxIterations, so a test driving the timeout needs a tolerance low enough that it does not converge early.

Related

#5924, #6065, #6216, PR #6055, PR #6214, PR #6222.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions