config reload: task tree reports success before plugin callbacks have run

[brbzull0]

traffic_ctl config reload reports a completed reload before plugin config callbacks have
run, and a plugin that rejects a config has no way to say so. Found while reviewing #13600;
rate_limit is used below as the probe, but this is not a rate_limit defect.

Impact

Plugin config callbacks work, and have since 2015 — TSMgmtUpdateRegister dispatch is
unaffected by this. What is missing is that the reload task tree neither waits for those
callbacks nor records their outcome, so the RPC asserts a completeness it cannot know.

Concretely: a plugin that rejects a config keeps its previous configuration, logs to
diags.log, and the reload still exits 0 with every subtask green. An operator who fixes a
malformed plugin config, reloads, and sees a green result has no signal that the running
configuration is not the one on disk. The callback's return value is discarded, so there is
nowhere for the plugin to put the answer.

Six global plugins register on master:

plugin	registration	can it reject a config today?
experimental/rate_limit	3-arg, file + mtime gate	yes — return false keeps the previous config
regex_revalidate	2-arg	no reject path surfaced
compress	2-arg	no — management_update() calls load_global_configuration() and returns 0
lua	2-arg	no reject path surfaced
stats_over_http	2-arg	no reject path surfaced
experimental/wasm	2-arg	no reject path surfaced

rate_limit is the only one passing a filename, so the only one gated on mtime; the others
fire on every reload whether or not their config changed. It is also the only one where the
tree currently reports a wrong answer rather than merely an incomplete one.

Since when. The dispatch is old — ConfigUpdateCbTable traces to 2015 — but it was not
wrong until there was a task tree to be absent from. #12892 (5bab268cb4, 2026-03-13)
introduced ReloadCoordinator, the per-task status model, and the reserve_subtask
protocol, wiring in FileManager, Configuration.cc, and traffic_server.cc. It did not
touch src/api/ConfigUpdateCbTable.cc or include/api/InkAPIInternal.h. Before that commit
the RPC made no granular claim; after it, it does.

Version:  master @ 956747452dcd36e8e56064aeaa47b0d8493ad0ac
Platform: Darwin 25.6.0, Apple clang 21.0.0
Config:   plugin.config -> rate_limit.so <path>/rate_limit.yaml

Proof

Two halves of one protocol; the plugin path has only the second.

ConfigRegistry::on_record_change() reserves before scheduling, and says why:

trafficserver/src/mgmt/config/ConfigRegistry.cc

Lines 165 to 174 in 9567474

	// Pre-register a CREATED subtask so the main task knows work is pending.
	// Without this, aggregate_status() can reach SUCCESS before the continuation
	// runs and creates the subtask. The continuation will activate the reserved
	// subtask instead of creating a new one.
	ReloadCoordinator::Get_Instance().reserve_subtask(ctx->config_key);
	// Schedule file reload on ET_TASK thread (always file-based, no rpc-supplied content)
	eventProcessor.schedule_imm(new RecordTriggeredReloadContinuation(ctx->mutex, ctx->config_key), ET_TASK);

  // Pre-register a CREATED subtask so the main task knows work is pending.
  // Without this, aggregate_status() can reach SUCCESS before the continuation
  // runs and creates the subtask.
  ReloadCoordinator::Get_Instance().reserve_subtask(ctx->config_key);
  eventProcessor.schedule_imm(new RecordTriggeredReloadContinuation(ctx->mutex, ctx->config_key), ET_TASK);

ConfigUpdateCbTable::invoke() schedules without reserving — exactly the case that comment
warns about:

trafficserver/src/api/ConfigUpdateCbTable.cc

Lines 75 to 79 in 9567474

	void
	ConfigUpdateCbTable::invoke(INKContInternal *contp)
	{
	eventProcessor.schedule_imm(new ConfigUpdateCallback(contp), ET_TASK);
	}

void
ConfigUpdateCbTable::invoke(INKContInternal *contp)
{
  eventProcessor.schedule_imm(new ConfigUpdateCallback(contp), ET_TASK);
}

ConfigUpdateCallback::event_handler then discards the callback's return value, so even a
convention-based failure signal would have nowhere to go:

trafficserver/include/api/InkAPIInternal.h

Lines 117 to 134 in 9567474

	int
	event_handler(int, void *)
	{
	if (m_cont->mutex) {
	MUTEX_TRY_LOCK(trylock, m_cont->mutex, this_ethread());
	if (!trylock.is_locked()) {
	eventProcessor.schedule_in(this, HRTIME_MSECONDS(10), ET_TASK);
	} else {
	m_cont->handleEvent(TS_EVENT_MGMT_UPDATE, nullptr);
	delete this;
	}
	} else {
	m_cont->handleEvent(TS_EVENT_MGMT_UPDATE, nullptr);
	delete this;
	}
	return 0;
	}

        m_cont->handleEvent(TS_EVENT_MGMT_UPDATE, nullptr);
        delete this;

Reproduction (rate_limit used as a probe)

A duplicate-SNI config, which takes rate_limit's existing non-throwing rejection path:

# rate_limit.yaml
selector:
  - sni: dup.example.com
    limit: 100
  - sni: dup.example.com
    limit: 200

The plugin rejects it and keeps the previous configuration:

[Aug 31 16:37:24.701] [ET_TASK 0] ERROR: [rate_limit] Duplicate SNIs being added (dup.example.com)
[Aug 31 16:37:24.701] [ET_TASK 0] ERROR: [rate_limit] Failed to reload YAML file: <path>/rate_limit.yaml

The same reload cycle reports success and exits 0:

$ traffic_ctl config reload -m -t rldtk-probe-1
✔ Reload scheduled [rldtk-probe-1]
✔ [rldtk-probe-1] ████████████████████ 1/1  success  (13ms)

Human:

$ traffic_ctl config status -t rldtk-probe-1
✔ Reload [success] — rldtk-probe-1
  Started : 2026 Aug 31 14:37:48.885
  Finished: 2026 Aug 31 14:37:48.898
  Duration: 13ms

  ✔ 1 success  ◌ 0 in-progress  ✗ 0 failed  (1 total)

  Tasks:
   ✔ ssl_ticket_key ·······························    0ms
      [Note]  SSL ticket key loading ...
      [Note]  SSL ticket key reloaded

JSON:

$ traffic_ctl config status -t rldtk-probe-1 --format json
{"jsonrpc": "2.0", "result": {"tasks": [{"config_token": "rldtk-probe-1", "status": "success",
 "description": "Main reload task - ...", "config_key": "", "filename": "",
 "meta": {"created_time_ms": "...", "last_updated_time_ms": "...", "main_task": "true"},
 "logs": [], "sub_tasks": [{"config_token": "rldtk-probe-1", "status": "success",
 "description": "ssl_ticket_key", "config_key": "ssl_ticket_key", "filename": "",
 "meta": {"created_time_ms": "...", "last_updated_time_ms": "...", "main_task": "false"},
 "logs": [{"level": "3", "text": "SSL ticket key loading ..."},
          {"level": "3", "text": "SSL ticket key reloaded"}], "sub_tasks": []}]}]},
 "id": "..."}

(timestamps, id, and absolute paths elided as ... / <path>)

ssl_ticket_key is the only subtask in either form — rate_limit never appears, in the
tree or in the exit code. The main task's last_updated_time_ms falls in the same
millisecond as the plugin's ERROR lines: the task closed while the callback was still
failing beside it. The 1/1 and the 13ms are the clearest statement of the problem —
the tree finished counting before the plugin was consulted.

Proposed change

Two defects, and they need fixing together rather than separately.

(a) No reservation. invoke() schedules on ET_TASK without reserve_subtask(), so
aggregate_status() reaches SUCCESS before the callback has run. Affects all six
registrants regardless of whether they can fail — it is what produces 1/1 success (13ms)
above with rate_limit absent from the tree.

(b) No failure channel. ConfigUpdateCallback::event_handler discards the callback's
return value, so a plugin that rejects a config has nowhere to report it.

Worth stating explicitly: fixing (a) alone would be worse than the current behaviour. A
reserved subtask with no way to fail can only ever be marked complete, so the tree would
show rate_limit: success for a config rate_limit had just rejected. Today it shows
nothing, which is at least not a false claim.

I have deliberately not proposed a patch, because the right shape depends on a decision I
should not make alone — whether TSMgmtUpdateRegister is worth extending at all:

• Extend it. Add reserve_subtask() in invoke() (the plugin name is already the
  cb_table key) and a failure channel — for example treating TS_EVENT_ERROR from the
  callback as failure and mapping it to ConfigContext::fail(). No new TSAPI, no enum
  change, no ABI break, and each existing plugin becomes fixable with a one-line change.
  Would need a sweep of the six to confirm none returns TS_EVENT_ERROR today for an
  unrelated reason.
• Replace it. [TS API] Add plugin API for config reload framework #13146 adds TSCfgRegister / TSCfgLoadCtx*, which gives migrated
  plugins both halves properly. If that is the intended direction, this issue becomes
  "migrate the six and deprecate TSMgmtUpdateRegister" once it lands. Noting it as a
  candidate, not a dependency — the defect above is present on master today either way.

Not done: no test. An autest asserting a non-zero exit from traffic_ctl config reload when
a plugin rejects its config would be the regression guard, and would fail today.