merge: sync Rongsong's scheduler-framework into chatbot
Full merge of origin/scheduler-framework (scheduler-core restructure,
failover-adoption, WAL_JOB_DISPATCH, apiserver backend health/heartbeat,
build-time versioning, bug-1673/1676/1678/1679/1680).
Conflict resolution:
- scheduler: adopt Rongsong's scheduler-core architecture (driver loop,
queue project/picker fields, resource bug-1569 fix)
- worker/controller_connector.c: keep Rongsong's reconnect/rejoin
structure, PORT our bug-1509 resend_job_status onto the rejoin success
path (his reconnect didn't re-report job state)
- apiserver backend: take Rongsong's health/heartbeat (supersedes our
e505066 circuit breaker)
- group.c: keep our bug-1513 comment
- misc: take theirs for additive fixes (go_build versioning,
test_group.c, persist.c RUSAGE_UPDATE, FilterRows bug-1680)
NOTE: our scheduler fixes (bug-1494 family dispatch-fail requeue,
PEND-reason classification) are NOT in Rongsong's branch — needs
verification/porting onto scheduler-core (separate follow-up).
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
commit
4e894d2184
395 changed files with 88517 additions and 5742 deletions
97
src/frontend/apiserver/backend/candidates.go
Normal file
97
src/frontend/apiserver/backend/candidates.go
Normal file
|
|
@ -0,0 +1,97 @@
|
|||
//go:build cgo
|
||||
|
||||
package backend
|
||||
|
||||
/*
|
||||
#include <stdlib.h>
|
||||
#include "xce/cluster.h"
|
||||
*/
|
||||
import "C"
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"unsafe"
|
||||
)
|
||||
|
||||
// Candidate is one parsed controller candidate: an optional logical name
|
||||
// plus the host:port it resolves to. Mirrors xce_candidate_t (xce/cluster.h).
|
||||
type Candidate struct {
|
||||
Name string
|
||||
Host string
|
||||
Port int
|
||||
}
|
||||
|
||||
// parseCandidates parses a comma-joined "[name=]host:port,..." candidate
|
||||
// list through the same shared C parsing+validation pair (xce_parse_peers
|
||||
// then xce_cluster_config_validate, both src/lib/cluster.c) that worker and
|
||||
// scheduler already call directly, in that same order, from their C
|
||||
// main.c's -- retiring apiserver's own hand-written parsePeer() in favor of
|
||||
// it (Req 6.1: apiserver accepts the identical candidate-list syntax
|
||||
// scheduler and worker already accept).
|
||||
//
|
||||
// xce_parse_peers alone is SYNTAX-only: it splits on ','/'='/':' but never
|
||||
// rejects a zero/non-numeric port (atoi() silently defaults malformed input
|
||||
// to 0) or an empty hostname. xce_cluster_config_validate is what actually
|
||||
// rejects those (src/lib/cluster.c:85-100) -- worker/scheduler always call
|
||||
// it immediately after a successful xce_parse_peers
|
||||
// (src/scheduler/src/main.c:300-309, src/worker/src/main.c:414-422), so
|
||||
// this wrapper must too, or apiserver would silently accept candidate
|
||||
// strings worker/scheduler reject (Req 6.2).
|
||||
// xce_cluster_config_validate also requires cfg->identity to be non-empty;
|
||||
// parseCandidates has no real identity to give it at this layer (clib.go's
|
||||
// RegisterToBackend assembles that separately, after parsing, from
|
||||
// operator-supplied From/Key config) -- a placeholder is stamped here
|
||||
// purely to satisfy that one precondition so the per-candidate
|
||||
// hostname/port checks actually run. It is never read or reused beyond
|
||||
// this call.
|
||||
//
|
||||
// On any malformed entry -- whether xce_parse_peers' own syntax check or
|
||||
// xce_cluster_config_validate's semantic check fails -- the WHOLE string is
|
||||
// rejected; neither function identifies which token was the bad one, and
|
||||
// neither does this wrapper (Req 6.2): that matches worker's and
|
||||
// scheduler's own established behavior, which likewise echoes the full
|
||||
// attempted --peers string back on any parse/validate failure (main.c in
|
||||
// both) rather than pinpointing one entry.
|
||||
//
|
||||
// If candidatesCount exceeds MAX_HA_CANDIDATES, xce_parse_peers silently
|
||||
// stops reading further tokens (no error, no warning at this layer) --
|
||||
// this wrapper does not add stricter behavior than the C functions have.
|
||||
func parseCandidates(peersStr string) ([]Candidate, error) {
|
||||
cPeers := C.CString(peersStr)
|
||||
defer C.free(unsafe.Pointer(cPeers))
|
||||
|
||||
malformedErr := func() error {
|
||||
return fmt.Errorf(
|
||||
"apiserver: --peers parse error: expected \"[name=]host:port,...\" got %q",
|
||||
peersStr)
|
||||
}
|
||||
|
||||
var cCfg C.xce_cluster_config_t
|
||||
if rc := C.xce_parse_peers(cPeers, &cCfg); rc != 0 {
|
||||
return nil, malformedErr()
|
||||
}
|
||||
|
||||
// Placeholder identity: satisfies xce_cluster_config_validate's
|
||||
// non-empty-identity precondition only -- see doc comment above. cCfg is
|
||||
// a fresh zero-valued struct, so the array is already NUL-padded past
|
||||
// what we write here.
|
||||
const placeholderIdentity = "apiserver-candidates-validate"
|
||||
for i := 0; i < len(placeholderIdentity); i++ {
|
||||
cCfg.identity[i] = C.char(placeholderIdentity[i])
|
||||
}
|
||||
if rc := C.xce_cluster_config_validate(&cCfg); rc != 0 {
|
||||
return nil, malformedErr()
|
||||
}
|
||||
|
||||
n := int(cCfg.candidates_count)
|
||||
out := make([]Candidate, n)
|
||||
for i := 0; i < n; i++ {
|
||||
cand := &cCfg.candidates[i]
|
||||
out[i] = Candidate{
|
||||
Name: C.GoString(&cand.name[0]),
|
||||
Host: C.GoString(&cand.hostname[0]),
|
||||
Port: int(cand.port),
|
||||
}
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
140
src/frontend/apiserver/backend/candidates_test.go
Normal file
140
src/frontend/apiserver/backend/candidates_test.go
Normal file
|
|
@ -0,0 +1,140 @@
|
|||
//go:build cgo
|
||||
|
||||
package backend
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// candidates_test.go exercises parseCandidates (candidates.go), the CGO
|
||||
// wrapper around the shared C parser xce_parse_peers() that replaces
|
||||
// apiserver's retired hand-written parsePeer() (Req 6.1, 6.2). Kept
|
||||
// //go:build cgo since parseCandidates only exists in the CGO build --
|
||||
// this file itself does not `import "C"` (a _test.go file can never do
|
||||
// that directly, golang/go#13938): it only calls the plain-Go-signature
|
||||
// parseCandidates defined in candidates.go, same split already used by
|
||||
// wire_dispatch_test.go/wire_dispatch_support.go in this package.
|
||||
|
||||
func TestParseCandidates_ValidHostPort(t *testing.T) {
|
||||
got, err := parseCandidates("ctrl01:9990")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
want := []Candidate{{Name: "", Host: "ctrl01", Port: 9990}}
|
||||
if len(got) != len(want) || got[0] != want[0] {
|
||||
t.Errorf("got %+v, want %+v", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseCandidates_NamePrefixSyntax(t *testing.T) {
|
||||
// The old hand-written parsePeer() could not parse this syntax at all
|
||||
// -- this is the format-parity case that proves real parity with
|
||||
// worker/scheduler's --peers (Req 6.1).
|
||||
got, err := parseCandidates("ctrl2=host2:9991")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
want := []Candidate{{Name: "ctrl2", Host: "host2", Port: 9991}}
|
||||
if len(got) != len(want) || got[0] != want[0] {
|
||||
t.Errorf("got %+v, want %+v", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseCandidates_MultipleCommaJoined(t *testing.T) {
|
||||
got, err := parseCandidates("ctrl01:9990,ctrl2=host2:9991,ctrl03:9992")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
want := []Candidate{
|
||||
{Name: "", Host: "ctrl01", Port: 9990},
|
||||
{Name: "ctrl2", Host: "host2", Port: 9991},
|
||||
{Name: "", Host: "ctrl03", Port: 9992},
|
||||
}
|
||||
if len(got) != len(want) {
|
||||
t.Fatalf("got %d candidates, want %d: %+v", len(got), len(want), got)
|
||||
}
|
||||
for i := range want {
|
||||
if got[i] != want[i] {
|
||||
t.Errorf("candidate[%d]: got %+v, want %+v", i, got[i], want[i])
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseCandidates_MalformedEntry_MissingColon(t *testing.T) {
|
||||
_, err := parseCandidates("ctrl01")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for missing colon, got nil")
|
||||
}
|
||||
// Req 6.2: reject identifying the malformed entry, consistent with how
|
||||
// worker/scheduler report it -- since xce_parse_peers() itself cannot
|
||||
// identify which token was malformed (same limit worker/scheduler's own
|
||||
// --peers parsing has), the whole attempted string is echoed back.
|
||||
if !strings.Contains(err.Error(), "ctrl01") {
|
||||
t.Errorf("error %q does not identify the malformed input", err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseCandidates_EmptyString(t *testing.T) {
|
||||
_, err := parseCandidates("")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for empty peers string, got nil")
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseCandidates_MalformedEntry_MidList(t *testing.T) {
|
||||
// A malformed second entry rejects the WHOLE list -- xce_parse_peers()
|
||||
// does not skip just the bad token and keep the rest (Req 6.2, matching
|
||||
// worker/scheduler's own all-or-nothing --peers parsing).
|
||||
_, err := parseCandidates("ctrl01:9990,badentry,ctrl03:9992")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for malformed mid-list entry, got nil")
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseCandidates_MalformedEntry_ZeroPort(t *testing.T) {
|
||||
// xce_parse_peers() alone would happily atoi() "0" into port=0 -- it is
|
||||
// xce_cluster_config_validate()'s port<=0 check (src/lib/cluster.c:94)
|
||||
// that rejects it, same as worker/scheduler (Req 6.2).
|
||||
_, err := parseCandidates("ctrl01:0")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for port 0, got nil")
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseCandidates_MalformedEntry_NonNumericPort(t *testing.T) {
|
||||
// atoi("abc") silently returns 0 -- caught by the same port<=0 check as
|
||||
// the explicit-zero case above, not by xce_parse_peers() itself.
|
||||
_, err := parseCandidates("ctrl01:abc")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for non-numeric port, got nil")
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseCandidates_MalformedEntry_EmptyHost(t *testing.T) {
|
||||
// xce_cluster_config_validate()'s hostname[0]=='\0' check
|
||||
// (src/lib/cluster.c:92) rejects an empty host, same as worker/scheduler.
|
||||
_, err := parseCandidates(":9990")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for empty host, got nil")
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseCandidates_OverCapSilentlyTruncates(t *testing.T) {
|
||||
// MAX_HA_CANDIDATES (src/include/config.h) is 4. xce_parse_peers()
|
||||
// silently stops reading further tokens once the cap is reached --
|
||||
// no error, no warning at this layer (the WARN-on-truncation behavior
|
||||
// belongs to xce_cluster_config_to_client_config(), a different
|
||||
// function, not exercised by this task). Don't invent stricter
|
||||
// behavior than the C function actually has.
|
||||
got, err := parseCandidates("c1:1,c2:2,c3:3,c4:4,c5:5")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if len(got) != 4 {
|
||||
t.Fatalf("got %d candidates, want 4 (MAX_HA_CANDIDATES cap)", len(got))
|
||||
}
|
||||
if got[3] != (Candidate{Name: "", Host: "c4", Port: 4}) {
|
||||
t.Errorf("candidate[3] = %+v, want the 4th entry (c5 silently dropped)", got[3])
|
||||
}
|
||||
}
|
||||
|
|
@ -12,6 +12,8 @@ package backend
|
|||
#include "idgen.h"
|
||||
#include "controller.capnp.h"
|
||||
#include "val_helpers.h"
|
||||
#include "xce/cluster.h"
|
||||
#include "xce/errnox.h"
|
||||
|
||||
// backend_ctx_t wraps xcecli_ctx_t for use as the C handler's arg.
|
||||
typedef struct {
|
||||
|
|
@ -25,36 +27,36 @@ static int xce_backend_handler(xce_request_t *req, void *arg) {
|
|||
return go_push_handler(req, arg);
|
||||
}
|
||||
|
||||
// go_backend_rejoin_handler is exported from Go (reconnect.go) and
|
||||
// registered below as the client library's xcecli_reconnect_cb
|
||||
// (component-failover-adoption task 4.3). It only sets a flag and returns
|
||||
// immediately -- see reconnect.go's own doc comment for why (the library
|
||||
// invokes it while still holding ac->failover.lock).
|
||||
extern int go_backend_rejoin_handler(xcecli_ctx_t, void*);
|
||||
|
||||
static int xce_backend_rejoin_handler(xcecli_ctx_t ctx, void *arg) {
|
||||
return go_backend_rejoin_handler(ctx, arg);
|
||||
}
|
||||
|
||||
// xce_backend_init initialises the client context, sets the handler,
|
||||
// starts the network loop, then joins as HostRole_apiserver.
|
||||
// starts the network loop, registers the rejoin callback, then joins as
|
||||
// HostRole_apiserver.
|
||||
static int xce_backend_init(xce_client_config_t *config, backend_ctx_t *ctx) {
|
||||
int rc = xcecli_ctx_init(&(ctx->ctx), config);
|
||||
if (rc < 0) return rc;
|
||||
xcecli_set_handler(ctx->ctx, (xce_request_handler_t)xce_backend_handler, NULL);
|
||||
rc = xcecli_start(ctx->ctx);
|
||||
if (rc < 0) return rc;
|
||||
// Registered AFTER xcecli_start() succeeds, not before
|
||||
// (component-failover-adoption task 4.3; matches worker's/scheduler's
|
||||
// task 2.3/3.1 fix): xcecli_start() unconditionally resets
|
||||
// ctx->failover.rejoin_cb to NULL as part of initializing per-context
|
||||
// failover state (client.c) -- registering before this call would be
|
||||
// silently wiped out immediately afterward.
|
||||
xcecli_set_reconnect_handler(ctx->ctx, xce_backend_rejoin_handler, NULL);
|
||||
return xcecli_join(ctx->ctx, HostRole_apiserver, NULL);
|
||||
}
|
||||
|
||||
// xce_backend_rejoin re-registers the apiserver with the (re)connected
|
||||
// controller. The controller resets its peer table on restart, so after a
|
||||
// transport reconnect the apiserver must re-join to resume push notifications
|
||||
// (component-failover-adoption Req 2.4). Call this from Go AFTER a heartbeat
|
||||
// has re-established the transport — never from the lib's own reconnect hook.
|
||||
static int xce_backend_rejoin(backend_ctx_t *ctx) {
|
||||
return xcecli_join(ctx->ctx, HostRole_apiserver, NULL);
|
||||
}
|
||||
|
||||
// xce_backend_force_reconnect force-reconnects the transport regardless of the
|
||||
// lib's internal state (see xcecli_force_reconnect). Used by the Go recovery
|
||||
// loop when the circuit breaker is degraded: the 30s heartbeat's fire-and-forget
|
||||
// reconnect only fires when ctx->state == XCE_CTX_ERR, and after a controller
|
||||
// restart whose election outlasts the FAILOVER_CALL retry budget the ctx can
|
||||
// stay RUN on a half-open connection, never reconnecting on its own.
|
||||
static int xce_backend_force_reconnect(backend_ctx_t *ctx) {
|
||||
return xcecli_force_reconnect(ctx->ctx);
|
||||
}
|
||||
|
||||
// Helpers to extract typed row pointers from a dataset_t.
|
||||
static xce_job_t **dataset_jobs(dataset_t *d) { return (xce_job_t **)d->rows; }
|
||||
static xce_worker_t **dataset_workers(dataset_t *d) { return (xce_worker_t **)d->rows; }
|
||||
|
|
@ -77,7 +79,9 @@ import (
|
|||
"apiserver/models"
|
||||
logger "apiserver/utils"
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
|
|
@ -89,6 +93,43 @@ import (
|
|||
// gCtx holds the C client context for the lifetime of the process.
|
||||
var gCtx C.backend_ctx_t
|
||||
|
||||
// gClusterCfg is the single validated xce_cluster_config_t RegisterToBackend
|
||||
// builds on every call -- either by parsing config.Peers via the shared C
|
||||
// parser (xce_parse_peers), or by synthesizing a one-candidate list from the
|
||||
// legacy config.Address/config.Port fields when Peers is empty (Req 1 AC5: a
|
||||
// single controller is the candidates_count=1 case of the identical
|
||||
// validate/convert/connect pipeline, not a separately-maintained code path
|
||||
// -- see buildClusterConfig() below).
|
||||
//
|
||||
// Package-level, not a buildClusterConfig()/RegisterToBackend()-local
|
||||
// variable, for the same reason gCtx and gIDCtx already are (process-lifetime
|
||||
// stable address): xce_cluster_config_to_client_config() (ConfigConverter,
|
||||
// src/lib/cluster.c) aliases gClientCfg.masters[i] directly at this struct's
|
||||
// own candidates[i].hostname fixed-char-array storage -- no strdup, no copy.
|
||||
// A caller that built the source xce_cluster_config_t on the stack (or any
|
||||
// Go-local variable) and then let gClientCfg outlive that scope would have a
|
||||
// use-after-scope bug: confirmed by reading src/lib/client.c's
|
||||
// xcecli_ctx_init() (`ctx->conf = cfg` -- stores a RAW pointer, never
|
||||
// copies) and src/lib/client_failover.c (`ac->conf->masters[next]` is
|
||||
// dereferenced on every failover rotation, not just at initial connect).
|
||||
// Worker's and scheduler's C implementations hit this identical hazard
|
||||
// (component-failover-adoption tasks 2.1/3.1) and solved it by keeping the
|
||||
// source config on a process-lifetime struct (worker: cc_ctx->cluster_cfg;
|
||||
// scheduler: sched->cluster_cfg) -- this package-level var is the direct Go
|
||||
// equivalent.
|
||||
var gClusterCfg C.xce_cluster_config_t
|
||||
|
||||
// gClientCfg is the xce_client_config_t ConfigConverter populates from
|
||||
// gClusterCfg and xce_backend_init() connects with. Package-level for the
|
||||
// same reason as gClusterCfg above: xcecli_ctx_init() stores gCtx.ctx->conf
|
||||
// as a raw pointer to whichever struct address is passed to it, dereferenced
|
||||
// for the ctx's entire runtime life (every failover rotation reads
|
||||
// ctx->conf->masters[]/nmasters, not only the initial connect) -- so this
|
||||
// struct's address must be stable for the process lifetime, not a
|
||||
// RegisterToBackend-local variable that goes out of scope when the function
|
||||
// returns.
|
||||
var gClientCfg C.xce_client_config_t
|
||||
|
||||
// gIDCtx is the C idgen context used to generate job IDs.
|
||||
var gIDCtx C.idgen_ctx_t
|
||||
var idCtxOnce sync.Once
|
||||
|
|
@ -97,44 +138,94 @@ var idCtxOnce sync.Once
|
|||
// Must be set via StartNotificationListener before RegisterToBackend.
|
||||
var gHandler NotificationHandler
|
||||
|
||||
func tryConnect(host string, port int, from, key string) error {
|
||||
cAddr := C.CString(host)
|
||||
cFrom := C.CString(from)
|
||||
cKey := C.CString(key)
|
||||
|
||||
var cConfig C.xce_client_config_t
|
||||
cConfig.nmasters = 1
|
||||
cConfig.masters[0] = cAddr
|
||||
cConfig.master_ports[0] = C.int(port)
|
||||
cConfig.from = cFrom
|
||||
cConfig.key = cKey
|
||||
|
||||
rc := C.xce_backend_init(&cConfig, &gCtx)
|
||||
if rc < 0 {
|
||||
// Init failed — the client did not retain the config, so the strings
|
||||
// are safe to free here (avoids a leak on the error path).
|
||||
C.free(unsafe.Pointer(cAddr))
|
||||
C.free(unsafe.Pointer(cFrom))
|
||||
C.free(unsafe.Pointer(cKey))
|
||||
return fmt.Errorf("xce_backend_init failed on %s:%d: %d", host, port, int(rc))
|
||||
// writeCCharArray copies s into dst, a fixed-size C char array field of
|
||||
// gClusterCfg (hostname/identity/key), truncating to len(dst)-1 bytes and
|
||||
// always leaving the result NUL-terminated. Because these are fixed arrays
|
||||
// embedded directly in gClusterCfg (a package-level, process-lifetime
|
||||
// struct -- see its doc comment), populating them this way needs no
|
||||
// C.CString/C.free bookkeeping at all: xce_cluster_config_to_client_config()
|
||||
// later aliases pointers directly into this same storage.
|
||||
func writeCCharArray(dst []C.char, s string) {
|
||||
n := len(s)
|
||||
if n > len(dst)-1 {
|
||||
n = len(dst) - 1
|
||||
}
|
||||
|
||||
// bug-1501: xcecli_ctx_init stores the config POINTER (ctx->conf = config),
|
||||
// not a copy — client_failover.c dereferences config->masters/from/key for
|
||||
// the client's ENTIRE lifetime (e.g. xce_failover_next_target strncpy's a
|
||||
// master host on every write-fail/redirect). The three C strings above must
|
||||
// therefore outlive this call: freeing them here (as the old defer did)
|
||||
// left config->masters[0] dangling, and a later failover read the freed
|
||||
// host → ASAN heap-use-after-free (strncpy, background C thread). They are
|
||||
// deliberately leaked for the process lifetime — tryConnect runs once at
|
||||
// startup and gCtx is a process-global.
|
||||
return nil
|
||||
for i := 0; i < n; i++ {
|
||||
dst[i] = C.char(s[i])
|
||||
}
|
||||
dst[n] = 0
|
||||
}
|
||||
|
||||
// RegisterToBackend initialises the XCE client library and joins as an API
|
||||
// server. StartNotificationListener should be called first so that push
|
||||
// events have a handler ready before the first push can arrive.
|
||||
func RegisterToBackend(config *BackendConfig) error {
|
||||
// formatCandidateList renders gClusterCfg's current candidates as
|
||||
// "host:port, host:port, ..." for diagnostic logging (Req 5.1, 5.2) -- the
|
||||
// same textual shape scheduler's sched_format_candidate_list() (scheduler.c)
|
||||
// produces, for cross-component log consistency.
|
||||
func formatCandidateList() string {
|
||||
n := int(gClusterCfg.candidates_count)
|
||||
parts := make([]string, 0, n)
|
||||
for i := 0; i < n; i++ {
|
||||
cand := &gClusterCfg.candidates[i]
|
||||
parts = append(parts, fmt.Sprintf("%s:%d", C.GoString(&cand.hostname[0]), int(cand.port)))
|
||||
}
|
||||
return strings.Join(parts, ", ")
|
||||
}
|
||||
|
||||
// currentClusterCandidates returns gClusterCfg's current candidates as a
|
||||
// Go-native slice. Exists so clib_test.go (a //go:build cgo test file that
|
||||
// cannot `import "C"` itself -- a Go-toolchain restriction, golang/go#13938)
|
||||
// can assert on buildClusterConfig()'s output shape without re-implementing
|
||||
// CGO field access in the test file, mirroring the candidates.go/
|
||||
// candidates_test.go split already established in this package.
|
||||
func currentClusterCandidates() []Candidate {
|
||||
n := int(gClusterCfg.candidates_count)
|
||||
out := make([]Candidate, n)
|
||||
for i := 0; i < n; i++ {
|
||||
cand := &gClusterCfg.candidates[i]
|
||||
out[i] = Candidate{
|
||||
Name: C.GoString(&cand.name[0]),
|
||||
Host: C.GoString(&cand.hostname[0]),
|
||||
Port: int(cand.port),
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// currentClientConfigNMasters exposes gClientCfg.nmasters as a plain int,
|
||||
// for the same test-file-cannot-import-C reason as currentClusterCandidates
|
||||
// above. Lets tests confirm xce_cluster_config_to_client_config() actually
|
||||
// propagated the full candidate count into the struct xce_backend_init()
|
||||
// connects with -- proving ONE call carries all N candidates, rather than N
|
||||
// separate single-candidate calls (the deleted per-peer Go loop's behavior,
|
||||
// which always hardcoded nmasters=1).
|
||||
func currentClientConfigNMasters() int {
|
||||
return int(gClientCfg.nmasters)
|
||||
}
|
||||
|
||||
// buildClusterConfig builds and validates the single xce_cluster_config_t
|
||||
// this connect attempt will use (Req 1 AC1-3/AC5), writing directly into the
|
||||
// package-level gClusterCfg (see its doc comment for why this must be
|
||||
// package-level rather than a local variable).
|
||||
//
|
||||
// A single controller is not a separately-maintained code path: when
|
||||
// config.Peers is non-empty it is parsed via the same shared C parser
|
||||
// (xce_parse_peers) worker and scheduler already call directly; otherwise a
|
||||
// one-candidate list is synthesized from config.Address/config.Port. Either
|
||||
// source is then run through the identical xce_cluster_config_validate()
|
||||
// call below -- no branch downstream of this function distinguishes the two
|
||||
// sources (Req 1 AC5), mirroring scheduler's sched_build_cluster_config()
|
||||
// (scheduler.c) and worker's equivalent (controller_connector.c).
|
||||
//
|
||||
// This intentionally does not reuse parseCandidates() (candidates.go):
|
||||
// parseCandidates() parses into its own function-local xce_cluster_config_t
|
||||
// and reconstructs a Go []Candidate slice from it -- exactly the
|
||||
// use-after-scope-prone, throwaway-C-struct pattern gClusterCfg's doc
|
||||
// comment above warns against for this call site. buildClusterConfig() must
|
||||
// parse directly into the package-level gClusterCfg so the very same struct
|
||||
// flows unmodified into xce_cluster_config_to_client_config() below.
|
||||
//
|
||||
// Does not call xce_cluster_config_to_client_config() or connect -- callers
|
||||
// (RegisterToBackend) do that separately.
|
||||
func buildClusterConfig(config *BackendConfig) error {
|
||||
from := config.From
|
||||
if from == "" {
|
||||
from = "xce_apiserver"
|
||||
|
|
@ -144,34 +235,88 @@ func RegisterToBackend(config *BackendConfig) error {
|
|||
key = "keepsecret$"
|
||||
}
|
||||
|
||||
gClusterCfg = C.xce_cluster_config_t{}
|
||||
|
||||
if len(config.Peers) > 0 {
|
||||
var lastErr error
|
||||
for _, peer := range config.Peers {
|
||||
host, port, err := parsePeer(peer)
|
||||
if err != nil {
|
||||
logger.Warn("Skipping invalid peer", zap.String("peer", peer), zap.Error(err))
|
||||
lastErr = err
|
||||
continue
|
||||
}
|
||||
if err := tryConnect(host, port, from, key); err != nil {
|
||||
logger.Warn("Candidate unreachable, trying next",
|
||||
zap.String("host", host), zap.Int("port", port), zap.Error(err))
|
||||
lastErr = err
|
||||
continue
|
||||
}
|
||||
logger.Info("Backend connection established (HA)",
|
||||
zap.String("host", host), zap.Int("port", port))
|
||||
return nil
|
||||
peersStr := strings.Join(config.Peers, ",")
|
||||
cPeers := C.CString(peersStr)
|
||||
rc := C.xce_parse_peers(cPeers, &gClusterCfg)
|
||||
C.free(unsafe.Pointer(cPeers))
|
||||
if rc != 0 {
|
||||
return fmt.Errorf(
|
||||
"apiserver: --peers parse error: expected \"[name=]host:port,...\" got %q",
|
||||
peersStr)
|
||||
}
|
||||
return fmt.Errorf("all HA candidates failed: %w", lastErr)
|
||||
} else {
|
||||
gClusterCfg.candidates_count = 1
|
||||
writeCCharArray(gClusterCfg.candidates[0].hostname[:], config.Address)
|
||||
gClusterCfg.candidates[0].port = C.int(config.Port)
|
||||
}
|
||||
|
||||
if err := tryConnect(config.Address, config.Port, from, key); err != nil {
|
||||
writeCCharArray(gClusterCfg.identity[:], from)
|
||||
writeCCharArray(gClusterCfg.key[:], key)
|
||||
|
||||
if rc := C.xce_cluster_config_validate(&gClusterCfg); rc != 0 {
|
||||
return fmt.Errorf(
|
||||
"apiserver: invalid controller candidate configuration (rc=%d): %s",
|
||||
int(rc), formatCandidateList())
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// RegisterToBackend initialises the XCE client library and joins as an API
|
||||
// server, using the full configured candidate list -- config.Peers if set,
|
||||
// otherwise a single-candidate list synthesized from config.Address/
|
||||
// config.Port (Req 1 AC5: the candidates_count=1 case of the identical
|
||||
// pipeline, not a separate code path -- see buildClusterConfig()). Builds
|
||||
// exactly ONE validated xce_cluster_config_t regardless of source, converts
|
||||
// it once via the shared ConfigConverter
|
||||
// (xce_cluster_config_to_client_config), and calls xce_backend_init()
|
||||
// exactly ONCE -- replacing the former dual-mechanism implementation (a
|
||||
// Go-level per-peer tryConnect() retry loop for the Peers case, and a
|
||||
// second, separately-coded tryConnect() call for the legacy Address/Port
|
||||
// case; both bypassed xce_cluster_config_t/xce_cluster_config_validate()/
|
||||
// xce_cluster_config_to_client_config() entirely, and both always
|
||||
// hardcoded nmasters=1).
|
||||
//
|
||||
// StartNotificationListener should be called first so that push events have
|
||||
// a handler ready before the first push can arrive.
|
||||
func RegisterToBackend(config *BackendConfig) error {
|
||||
if err := buildClusterConfig(config); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
logger.Info("Attempting backend connection",
|
||||
zap.Int("candidates", int(gClusterCfg.candidates_count)),
|
||||
zap.String("candidate_list", formatCandidateList()))
|
||||
|
||||
gClientCfg = C.xce_client_config_t{}
|
||||
if rc := C.xce_cluster_config_to_client_config(&gClusterCfg, &gClientCfg); rc != 0 {
|
||||
return fmt.Errorf("apiserver: candidate list conversion failed (rc=%d)", int(rc))
|
||||
}
|
||||
// ConfigConverter deliberately does not set from/key (caller-owned
|
||||
// identity/auth, per its own doc comment) -- point them at gClusterCfg's
|
||||
// own identity/key fixed arrays rather than allocating separate C
|
||||
// strings: gClusterCfg is the same package-level, process-lifetime
|
||||
// struct ConfigConverter already aliased gClientCfg.masters[i] into, so
|
||||
// this needs no additional C.CString/C.free bookkeeping.
|
||||
gClientCfg.from = &gClusterCfg.identity[0]
|
||||
gClientCfg.key = &gClusterCfg.key[0]
|
||||
|
||||
rc := C.xce_backend_init(&gClientCfg, &gCtx)
|
||||
if rc < 0 {
|
||||
logger.Warn("All configured backend candidates failed",
|
||||
zap.Int("candidates", int(gClusterCfg.candidates_count)),
|
||||
zap.String("candidate_list", formatCandidateList()),
|
||||
zap.Int("rc", int(rc)))
|
||||
return fmt.Errorf("xce_backend_init failed across %d candidate(s) (rc=%d): %s",
|
||||
int(gClusterCfg.candidates_count), int(rc), formatCandidateList())
|
||||
}
|
||||
|
||||
logger.Info("Backend connection established",
|
||||
zap.String("address", config.Address),
|
||||
zap.Int("port", config.Port))
|
||||
zap.Int("nmasters", int(gClientCfg.nmasters)),
|
||||
zap.String("candidate_list", formatCandidateList()))
|
||||
return nil
|
||||
}
|
||||
|
||||
|
|
@ -206,69 +351,35 @@ func Cleanup() {
|
|||
C.idgen_ctx_fini(&gIDCtx)
|
||||
}
|
||||
|
||||
// rejoinBackend re-registers the apiserver with the controller after a
|
||||
// transport reconnect (component-failover-adoption Req 2.4). xcecli_heartbeat
|
||||
// is fire-and-forget: it best-effort reconnects the transport but never
|
||||
// re-joins, so without an explicit rejoin the controller's restarted peer
|
||||
// table never learns this apiserver again and push notifications stop.
|
||||
func rejoinBackend() {
|
||||
if gCtx.ctx == nil {
|
||||
return
|
||||
// currentReconnectGen reads endpoint_t.reconnect_gen (netio.h) off the
|
||||
// backend's live endpoint via the existing, previously-unused C accessor
|
||||
// xcecli_get_my_endpoint() -- no new C getter needed (component-failover-
|
||||
// adoption Phase 6, bug-1548). Uses an explicit atomic load (rather than a
|
||||
// bare field dereference) so the Go-side read is formally atomic and pairs
|
||||
// correctly with the C11 _Atomic write in xce_ctx_reconnect() (xcectx.c).
|
||||
func currentReconnectGen() uint64 {
|
||||
ep := C.xcecli_get_my_endpoint(gCtx.ctx)
|
||||
if ep == nil {
|
||||
return 0
|
||||
}
|
||||
rc := C.xce_backend_rejoin(&gCtx)
|
||||
if rc < 0 {
|
||||
logger.Warn("backend rejoin failed (will retry on next heartbeat)",
|
||||
zap.Int("rc", int(rc)))
|
||||
return
|
||||
}
|
||||
logger.Info("backend re-joined controller after reconnect")
|
||||
return atomic.LoadUint64((*uint64)(unsafe.Pointer(&ep.reconnect_gen)))
|
||||
}
|
||||
|
||||
// StartHeartbeat launches a goroutine that sends periodic xcecli_heartbeat
|
||||
// keep-alive messages to the controller. Stops when ctx is cancelled. When a
|
||||
// heartbeat transitions from failure to success (the transport reconnect
|
||||
// succeeded), it triggers an explicit backend rejoin so the apiserver resumes
|
||||
// receiving push notifications without a process restart.
|
||||
// keep-alive messages to the controller. Stops when ctx is cancelled.
|
||||
//
|
||||
// Wires runHeartbeat's reconnect-detected and every-successful-tick hooks to
|
||||
// reconnect.go's setNeedsRejoin/checkAndPerformRejoin (task 4.3) -- closing
|
||||
// the idle-period reconnect gap (task 4.4/Phase 6, Req 2 AC4/AC5):
|
||||
// XCECLI_FF_CALL (backing xcecli_heartbeat) never invokes the client
|
||||
// library's rejoin callback, so nothing else would set the pending-rejoin
|
||||
// flag if this heartbeat prober is the only traffic during a controller
|
||||
// outage. currentReconnectGen (above) is what actually detects the
|
||||
// reconnect even when it completed silently inside a single XCECLI_FF_CALL.
|
||||
func StartHeartbeat(ctx context.Context) {
|
||||
var prevFailed atomic.Bool
|
||||
go runHeartbeat(ctx, func(secs int) {
|
||||
rc := C.xcecli_heartbeat(gCtx.ctx, C.int(secs))
|
||||
if needsRejoinAfterHeartbeat(rc >= 0, &prevFailed) {
|
||||
rejoinBackend()
|
||||
MarkHealthy()
|
||||
}
|
||||
})
|
||||
go recoveryLoop(ctx)
|
||||
}
|
||||
|
||||
// recoveryLoop retries a forced transport reconnect + rejoin on a short
|
||||
// interval while the circuit breaker is degraded (bug-1513 follow-up). The
|
||||
// 30s heartbeat's fire-and-forget reconnect only fires when the lib's internal
|
||||
// state is ERR; after a controller restart whose election outlasts the
|
||||
// FAILOVER_CALL retry budget the ctx can stay RUN on a half-open connection and
|
||||
// the heartbeat never reconnects — the backend stays degraded until a manual
|
||||
// restart. This loop force-reconnects deterministically and re-joins, so a
|
||||
// full controller death+restart recovers on its own.
|
||||
func recoveryLoop(ctx context.Context) {
|
||||
t := time.NewTicker(2 * time.Second)
|
||||
defer t.Stop()
|
||||
for {
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return
|
||||
case <-t.C:
|
||||
if !IsDegraded() {
|
||||
continue
|
||||
}
|
||||
rc := C.xce_backend_force_reconnect(&gCtx)
|
||||
if rc < 0 {
|
||||
// Still unreachable (controller electing/booting) — retry next tick.
|
||||
continue
|
||||
}
|
||||
rejoinBackend()
|
||||
MarkHealthy()
|
||||
}
|
||||
}
|
||||
go runHeartbeat(ctx, func(secs int) int {
|
||||
return int(C.xcecli_heartbeat(gCtx.ctx, C.int(secs)))
|
||||
}, currentReconnectGen, setNeedsRejoin, checkAndPerformRejoin)
|
||||
}
|
||||
|
||||
// go_push_handler is called by the C library on every incoming request.
|
||||
|
|
@ -443,47 +554,83 @@ func handleNotifyData(nd *C.xce_notify_data_t) C.int {
|
|||
return 0
|
||||
}
|
||||
|
||||
// isBackendUnavailable reports whether a negative xcecli_* return means the
|
||||
// backend controller cannot serve the op (transport failure OR an XCE
|
||||
// not-ready/masterless/standby code) rather than a semantic rejection. Only
|
||||
// these trip the Req 4 circuit breaker — a semantic rejection (-EINVAL/
|
||||
// -ENOENT/...) must never mark the backend "down".
|
||||
func isBackendUnavailable(rc int) bool {
|
||||
switch rc {
|
||||
case -int(C.ECONNRESET), -int(C.ECONNREFUSED), -int(C.EHOSTUNREACH),
|
||||
-int(C.ENETUNREACH), -int(C.ENETDOWN), -int(C.ETIMEDOUT),
|
||||
-int(C.EPIPE), -int(C.EIO), -int(C.ENOTCONN), -int(C.ESHUTDOWN),
|
||||
-int(C.ENODEV), -int(C.ENOLINK), -int(C.ECONNABORTED):
|
||||
return true
|
||||
// XCE controller role codes (errnox.h): the controller is reachable but
|
||||
// cannot authoritatively serve the op — no master, redirecting, initializing/
|
||||
// electing, or a standby. All mean "backend unavailable right now".
|
||||
case -(int(C.E_XCE_NOMASTER)), -(int(C.E_XCE_REDIRECT)),
|
||||
-(int(C.E_XCE_NOT_READY)), -(int(C.E_XCE_STANDBY)):
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
// ErrBackendUnavailable is returned immediately by callBackend (and
|
||||
// therefore by every wrapper below) when ApiserverBackendHealth reports the
|
||||
// backend connection degraded (Req 4.1) -- a clear, distinct error a caller
|
||||
// can recognize as "backend down, retrying" rather than an unrelated
|
||||
// internal failure, without ever invoking the underlying blocking
|
||||
// C.xcecli_* call.
|
||||
var ErrBackendUnavailable = errors.New(
|
||||
"apiserver: backend connection unavailable (circuit breaker open); try again shortly")
|
||||
|
||||
// callBackend guards a C.xcecli_* write call with the fail-fast circuit
|
||||
// breaker (component-failover-adoption Req 4): when degraded, return a
|
||||
// service-unavailable error immediately rather than invoking a call that would
|
||||
// hang on a dead controller. A transport failure trips the breaker so the NEXT
|
||||
// call fails fast (the lib's retry budget already bounded this call's block).
|
||||
// fn returns the (negative) xcecli return code converted to a plain int so the
|
||||
// breaker logic is unit-testable without cgo.
|
||||
func callBackend(name string, fn func() int) error {
|
||||
// backendDegradedTTL is how long callBackend marks the backend degraded
|
||||
// (ApiserverBackendHealth.MarkDegraded) after a call's own outcome
|
||||
// indicates its retry budget was exhausted, before request handlers resume
|
||||
// probing it directly again -- the half-open half of the circuit breaker
|
||||
// (design.md's ApiserverBackendHealth). Chosen well below xcecli's own
|
||||
// full per-call retry budget (XCECLI_FAILOVER_CALL: max_retries=20,
|
||||
// backoff capped at 5000ms -- roughly 1-2 minutes total,
|
||||
// client_failover.c:218,220): 10s is short enough that a backend which
|
||||
// recovers quickly gets re-probed promptly, while still long enough that
|
||||
// every request arriving during a genuine outage fails fast instead of
|
||||
// each independently re-discovering the same ~100s exhausted budget one
|
||||
// blocking call at a time.
|
||||
const backendDegradedTTL = 10 * time.Second
|
||||
|
||||
// notReadyRC exposes -E_XCE_NOT_READY as a plain Go int, for
|
||||
// health_rejoin_test.go (a //go:build cgo test file that cannot `import "C"`
|
||||
// itself, golang/go#13938) to script a callBackend closure returning this
|
||||
// exact retry-budget-exhaustion value without hardcoding the raw errno
|
||||
// number -- same plain-Go-signature test-seam rationale as performRejoinFn
|
||||
// (reconnect.go) and currentClientConfigNMasters() (above).
|
||||
var notReadyRC = -int(C.E_XCE_NOT_READY)
|
||||
|
||||
// callBackend is the single shared pre-call/post-call helper every
|
||||
// gCtx.ctx-based backend-call wrapper below (SubmitJob, ControlJob,
|
||||
// ControlQueue, ControlHost, FetchFile) calls around its own single
|
||||
// C.xcecli_* invocation (component-failover-adoption task 4.5, boundary:
|
||||
// ApiserverBackendBridge/ApiserverBackendHealth) -- implemented once here,
|
||||
// not copy-pasted per wrapper. fn performs exactly one C.xcecli_* call and
|
||||
// returns its raw rc as a plain Go int (not C.int) -- the same
|
||||
// plain-Go-signature test seam pattern as reconnect.go's performRejoinFn,
|
||||
// letting a //go:build cgo test file (which can never `import "C"` itself,
|
||||
// golang/go#13938) drive this helper directly with a scripted closure, with
|
||||
// no live backend connection or even an initialized gCtx required.
|
||||
//
|
||||
// Pre-call (Req 4.1): if IsDegraded(), fn is never invoked at all -- the
|
||||
// wrapped call (and its own up-to-~100s xcecli retry budget) is skipped
|
||||
// entirely, returning ErrBackendUnavailable immediately. checkAndPerformRejoin
|
||||
// is deliberately NOT called on this path either: it can itself block on
|
||||
// xcecli_join's own up-to-~100s retry budget, which would defeat the whole
|
||||
// point of failing fast here (Req 4.1's bounded, sub-second response). The
|
||||
// rejoin-retry cadence during this window keeps running regardless, driven
|
||||
// by heartbeat.go's own unconditional per-tick call to checkAndPerformRejoin
|
||||
// (task 4.4) -- which never reads IsDegraded() -- so the pending-rejoin
|
||||
// flag's retry-until-actually-joined behavior is never suppressed by this
|
||||
// short-circuit (see health.go's own doc comment for why these two states
|
||||
// must stay independent).
|
||||
//
|
||||
// Post-call (only reached when not degraded): the existing rejoin check
|
||||
// (checkAndPerformRejoin, task 4.3/4.4, unchanged) always runs, regardless
|
||||
// of fn's own outcome -- a pending rejoin is orthogonal to whether this
|
||||
// particular call happened to succeed. If fn's rc is exactly
|
||||
// -E_XCE_NOT_READY (this call's own retry budget exhausted), MarkDegraded
|
||||
// opens the circuit for subsequent calls.
|
||||
func callBackend(name string, fn func() int) (rc int, err error) {
|
||||
if IsDegraded() {
|
||||
return fmt.Errorf("%s: backend controller unavailable (reconnecting)", name)
|
||||
return 0, ErrBackendUnavailable
|
||||
}
|
||||
|
||||
rc = fn()
|
||||
checkAndPerformRejoin()
|
||||
|
||||
if rc == notReadyRC {
|
||||
MarkDegraded(backendDegradedTTL)
|
||||
}
|
||||
rc := fn()
|
||||
if rc < 0 {
|
||||
if isBackendUnavailable(rc) {
|
||||
MarkDegraded()
|
||||
}
|
||||
return fmt.Errorf("%s failed: %d", name, rc)
|
||||
return rc, fmt.Errorf("%s failed: %d", name, rc)
|
||||
}
|
||||
return nil
|
||||
return rc, nil
|
||||
}
|
||||
|
||||
// SubmitJob sends a job submission to the backend via xcecli_jobsubmit.
|
||||
|
|
@ -497,9 +644,10 @@ func SubmitJob(job *models.JobInfo, arraySize int) error {
|
|||
js.array_size = C.uint64_t(arraySize)
|
||||
|
||||
var outJobID C.uint64_t
|
||||
if err := callBackend("xcecli_jobsubmit", func() int {
|
||||
_, err := callBackend("xcecli_jobsubmit", func() int {
|
||||
return int(C.xcecli_jobsubmit(gCtx.ctx, js, &outJobID))
|
||||
}); err != nil {
|
||||
})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if outJobID != 0 {
|
||||
|
|
@ -516,12 +664,7 @@ func SubmitJob(job *models.JobInfo, arraySize int) error {
|
|||
func ControlJob(jobid uint64, action int, content string) error {
|
||||
var ctrl C.xce_jobctrl_t
|
||||
ctrl.jobid = C.uint64_t(jobid)
|
||||
// ctrl.action is the capnp JobControl union DISCRIMINANT (0=siginfo,
|
||||
// 1=ckptoption), NOT the models.JOB_* constant. bug-1490: setting it to
|
||||
// JOB_STOP/RESUME/KILL/CHKPNT (1/2/3/8) made the controller's job_ctrl()
|
||||
// select the wrong union branch (or none) and reject every control with
|
||||
// -EINVAL, so hold/release/cancel never reached the job.
|
||||
ctrl.action = C.int(jobCtrlDiscriminant(action))
|
||||
ctrl.action = C.int(action)
|
||||
|
||||
// For signal-based actions set signo; for checkpoint set ckpt options.
|
||||
// xce_jobctrl_t.u is a C union — we use a static C helper to set the
|
||||
|
|
@ -538,12 +681,10 @@ func ControlJob(jobid uint64, action int, content string) error {
|
|||
C.jobctrl_set_ckptoption(&ctrl, &ckpt)
|
||||
}
|
||||
|
||||
if err := callBackend("xcecli_jobctrl", func() int {
|
||||
_, err := callBackend("xcecli_jobctrl", func() int {
|
||||
return int(C.xcecli_jobctrl(gCtx.ctx, &ctrl))
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
})
|
||||
return err
|
||||
}
|
||||
|
||||
// ControlQueue sends a queue open/close/activate/deactivate action to the
|
||||
|
|
@ -557,12 +698,10 @@ func ControlQueue(queuename string, action int) error {
|
|||
queue: cQueue,
|
||||
}
|
||||
|
||||
if err := callBackend("xcecli_queuectrl", func() int {
|
||||
_, err := callBackend("xcecli_queuectrl", func() int {
|
||||
return int(C.xcecli_queuectrl(gCtx.ctx, &ctrl))
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
})
|
||||
return err
|
||||
}
|
||||
|
||||
// ControlHost sends a host open/close action to the backend via xcecli_hostctrl.
|
||||
|
|
@ -575,12 +714,10 @@ func ControlHost(hostname string, action int) error {
|
|||
hostname: cHost,
|
||||
}
|
||||
|
||||
if err := callBackend("xcecli_hostctrl", func() int {
|
||||
_, err := callBackend("xcecli_hostctrl", func() int {
|
||||
return int(C.xcecli_hostctrl(gCtx.ctx, &ctrl))
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
})
|
||||
return err
|
||||
}
|
||||
|
||||
// signalNumber maps a job action and optional signal name to a POSIX signal
|
||||
|
|
@ -606,36 +743,38 @@ func signalNumber(signame string, action int) int {
|
|||
// controller→worker chain. The filepath must be the fully resolved absolute
|
||||
// path on the exec host.
|
||||
func FetchFile(jobid uint64, filepath string, offset uint64, maxBytes uint32) (*FetchFileResult, error) {
|
||||
cFilepath := C.CString(filepath)
|
||||
defer C.free(unsafe.Pointer(cFilepath))
|
||||
cFilepath := C.CString(filepath)
|
||||
defer C.free(unsafe.Pointer(cFilepath))
|
||||
|
||||
msg := C.xce_fetchfile_t{
|
||||
jobid: C.uint64_t(jobid),
|
||||
filepath: cFilepath,
|
||||
offset: C.uint64_t(offset),
|
||||
max_bytes: C.uint32_t(maxBytes),
|
||||
}
|
||||
msg := C.xce_fetchfile_t{
|
||||
jobid: C.uint64_t(jobid),
|
||||
filepath: cFilepath,
|
||||
offset: C.uint64_t(offset),
|
||||
max_bytes: C.uint32_t(maxBytes),
|
||||
}
|
||||
|
||||
var reply *C.xce_fetchfile_reply_t
|
||||
rc := C.xcecli_fetchfile(gCtx.ctx, &msg, &reply)
|
||||
if rc < 0 {
|
||||
return nil, fmt.Errorf("xcecli_fetchfile failed: %d", int(rc))
|
||||
}
|
||||
if reply == nil {
|
||||
return nil, fmt.Errorf("xcecli_fetchfile returned nil reply")
|
||||
}
|
||||
defer C.free(unsafe.Pointer(reply))
|
||||
var reply *C.xce_fetchfile_reply_t
|
||||
_, err := callBackend("xcecli_fetchfile", func() int {
|
||||
return int(C.xcecli_fetchfile(gCtx.ctx, &msg, &reply))
|
||||
})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if reply == nil {
|
||||
return nil, fmt.Errorf("xcecli_fetchfile returned nil reply")
|
||||
}
|
||||
defer C.free(unsafe.Pointer(reply))
|
||||
|
||||
if reply.error != 0 {
|
||||
return nil, fmt.Errorf("fetchfile remote error: %d", int(reply.error))
|
||||
}
|
||||
if reply.error != 0 {
|
||||
return nil, fmt.Errorf("fetchfile remote error: %d", int(reply.error))
|
||||
}
|
||||
|
||||
result := &FetchFileResult{
|
||||
FileSize: uint64(reply.file_size),
|
||||
BytesRead: uint32(reply.bytes_read),
|
||||
}
|
||||
if reply.bytes_read > 0 && reply.content != nil {
|
||||
result.Content = C.GoBytes(unsafe.Pointer(reply.content), C.int(reply.bytes_read))
|
||||
}
|
||||
return result, nil
|
||||
result := &FetchFileResult{
|
||||
FileSize: uint64(reply.file_size),
|
||||
BytesRead: uint32(reply.bytes_read),
|
||||
}
|
||||
if reply.bytes_read > 0 && reply.content != nil {
|
||||
result.Content = C.GoBytes(unsafe.Pointer(reply.content), C.int(reply.bytes_read))
|
||||
}
|
||||
return result, nil
|
||||
}
|
||||
|
|
|
|||
288
src/frontend/apiserver/backend/clib_test.go
Normal file
288
src/frontend/apiserver/backend/clib_test.go
Normal file
|
|
@ -0,0 +1,288 @@
|
|||
//go:build cgo
|
||||
|
||||
package backend
|
||||
|
||||
import (
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"go.uber.org/zap/zaptest/observer"
|
||||
)
|
||||
|
||||
// clib_test.go exercises RegisterToBackend/buildClusterConfig (clib.go), the
|
||||
// unified candidate-list connect pipeline that replaces the former
|
||||
// dual-mechanism implementation (component-failover-adoption, task 4.2,
|
||||
// boundary: ApiserverBackendBridge). Kept //go:build cgo since these
|
||||
// functions only exist in the CGO build -- this file itself does not
|
||||
// `import "C"` (a _test.go file can never do that directly, golang/go#13938):
|
||||
// it only calls the plain-Go-signature functions defined in clib.go, same
|
||||
// split candidates.go/candidates_test.go already established in this
|
||||
// package.
|
||||
//
|
||||
// Test strategy:
|
||||
// - Req 1.5 (no-Peers and single-entry-Peers converge on the identical
|
||||
// validated xce_cluster_config_t shape) and Req 1.1-1.3 (multi-candidate
|
||||
// passthrough) are exercised via buildClusterConfig() directly -- pure
|
||||
// struct-building + validation, no network I/O, so these run fast and
|
||||
// avoid any socket/leak concerns from a real (failing) connect attempt.
|
||||
// - "a single xce_backend_init() call carries the full candidate count"
|
||||
// is exercised by calling the real RegisterToBackend() against closed
|
||||
// local ports (connect() returns ECONNREFUSED near-instantly, same
|
||||
// "no backoff loop inside xcecli_start()'s per-master connect scan"
|
||||
// property scheduler's test_scheduler_connect.c and worker's task 2.3
|
||||
// test both rely on) and inspecting gClientCfg.nmasters afterward via
|
||||
// currentClientConfigNMasters() -- this is deliberately the ONLY test in
|
||||
// this file that reaches xce_backend_init()/a real connect() call, to
|
||||
// keep the (unavoidable, pre-existing -- see status report CONCERNS)
|
||||
// per-call xcecli_ctx_init() allocation this path leaks on a failed
|
||||
// connect bounded to one occurrence.
|
||||
// - "no-peers and single-entry-peers produce the identical connect log
|
||||
// sequence" is exercised behaviorally via captureLog() (handler_test.go,
|
||||
// this package), asserting on the log MESSAGE sequence (not field
|
||||
// values) RegisterToBackend emits for each source.
|
||||
// - The "source config structs are package-level, not stack-local"
|
||||
// regression guard is a source-text scan of clib.go, same genre as
|
||||
// scheduler's/worker's structural regression tests (test_scheduler_connect.c,
|
||||
// test_controller_connector.c) for the identical hazard.
|
||||
|
||||
// testPortBase is deliberately unlikely to have a real listener bound in
|
||||
// CI/dev sandboxes -- connect() against a closed local port fails fast, so
|
||||
// RegisterToBackend() calls below return quickly without needing a live
|
||||
// controller (same precedent as scheduler's TEST_PORT_BASE).
|
||||
const testPortBase = 28581
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// Req 1.5 -- unified candidate-list pipeline: no branch for "single host"
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
func TestBuildClusterConfig_NoPeersAndSingleEntryPeers_IdenticalShape(t *testing.T) {
|
||||
// (a) No Peers: legacy Address/Port synthesized into a single candidate.
|
||||
cfgNoPeers := &BackendConfig{
|
||||
Address: "127.0.0.1",
|
||||
Port: testPortBase,
|
||||
From: "xce_apiserver_test",
|
||||
Key: "keepsecret$",
|
||||
}
|
||||
if err := buildClusterConfig(cfgNoPeers); err != nil {
|
||||
t.Fatalf("buildClusterConfig(no peers) failed: %v", err)
|
||||
}
|
||||
noPeersCandidates := currentClusterCandidates()
|
||||
|
||||
// (b) A single-entry Peers list pointed at the exact same host:port.
|
||||
cfgOnePeer := &BackendConfig{
|
||||
Peers: []string{"127.0.0.1:28581"},
|
||||
From: "xce_apiserver_test",
|
||||
Key: "keepsecret$",
|
||||
}
|
||||
if err := buildClusterConfig(cfgOnePeer); err != nil {
|
||||
t.Fatalf("buildClusterConfig(one peer) failed: %v", err)
|
||||
}
|
||||
onePeerCandidates := currentClusterCandidates()
|
||||
|
||||
if len(noPeersCandidates) != 1 {
|
||||
t.Fatalf("no-peers: got %d candidates, want 1", len(noPeersCandidates))
|
||||
}
|
||||
if len(onePeerCandidates) != 1 {
|
||||
t.Fatalf("single-entry-peers: got %d candidates, want 1", len(onePeerCandidates))
|
||||
}
|
||||
if noPeersCandidates[0].Host != onePeerCandidates[0].Host ||
|
||||
noPeersCandidates[0].Port != onePeerCandidates[0].Port {
|
||||
t.Errorf("no-peers and single-entry-peers candidates diverge: %+v vs %+v",
|
||||
noPeersCandidates[0], onePeerCandidates[0])
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildClusterConfig_MultiCandidatePassthrough(t *testing.T) {
|
||||
cfg := &BackendConfig{
|
||||
Peers: []string{
|
||||
"127.0.0.1:28590",
|
||||
"127.0.0.1:28591",
|
||||
"127.0.0.1:28592",
|
||||
},
|
||||
From: "xce_apiserver_test",
|
||||
Key: "keepsecret$",
|
||||
}
|
||||
if err := buildClusterConfig(cfg); err != nil {
|
||||
t.Fatalf("buildClusterConfig failed: %v", err)
|
||||
}
|
||||
candidates := currentClusterCandidates()
|
||||
if len(candidates) != 3 {
|
||||
t.Fatalf("got %d candidates, want 3", len(candidates))
|
||||
}
|
||||
for i, want := range []int{28590, 28591, 28592} {
|
||||
if candidates[i].Host != "127.0.0.1" || candidates[i].Port != want {
|
||||
t.Errorf("candidate %d = %+v, want host=127.0.0.1 port=%d", i, candidates[i], want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildClusterConfig_MalformedPeersRejected(t *testing.T) {
|
||||
cfg := &BackendConfig{
|
||||
Peers: []string{"not-a-valid-entry"},
|
||||
From: "xce_apiserver_test",
|
||||
Key: "keepsecret$",
|
||||
}
|
||||
err := buildClusterConfig(cfg)
|
||||
if err == nil {
|
||||
t.Fatal("expected error for malformed --peers entry, got nil")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "not-a-valid-entry") {
|
||||
t.Errorf("error %q does not identify the malformed input", err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// Req 1.1-1.3 -- one xce_backend_init() call carries the full candidate list
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
func TestRegisterToBackend_MultiCandidate_SingleInitCallCarriesFullCount(t *testing.T) {
|
||||
captureLog(t) // RegisterToBackend logs via utils.Log; nil otherwise (utils.InitLogger is never called in unit tests)
|
||||
|
||||
cfg := &BackendConfig{
|
||||
Peers: []string{
|
||||
"127.0.0.1:28600",
|
||||
"127.0.0.1:28601",
|
||||
"127.0.0.1:28602",
|
||||
},
|
||||
From: "xce_apiserver_test",
|
||||
Key: "keepsecret$",
|
||||
}
|
||||
|
||||
start := time.Now()
|
||||
err := RegisterToBackend(cfg)
|
||||
elapsed := time.Since(start)
|
||||
|
||||
if err == nil {
|
||||
t.Fatal("expected RegisterToBackend to fail against 3 closed local ports")
|
||||
}
|
||||
// Generous bound: closed-port ECONNREFUSED is near-instant, well under a
|
||||
// few seconds even across 3 candidates -- proves this is NOT the deleted
|
||||
// per-candidate Go loop silently retrying/blocking somewhere.
|
||||
if elapsed > 5*time.Second {
|
||||
t.Errorf("RegisterToBackend took %v against closed ports -- too slow, "+
|
||||
"suggests a retry loop is still present", elapsed)
|
||||
}
|
||||
|
||||
// The struct xce_backend_init() was actually called with must carry the
|
||||
// FULL candidate count (nmasters=3), not the hardcoded 1 the deleted
|
||||
// per-peer Go loop always used -- proves ONE call, not three separate
|
||||
// single-candidate calls.
|
||||
if got := currentClientConfigNMasters(); got != 3 {
|
||||
t.Errorf("gClientCfg.nmasters = %d, want 3 (a single call must carry "+
|
||||
"the full candidate count)", got)
|
||||
}
|
||||
}
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// Req 5.1/5.2 -- no-peers and single-entry-peers produce the identical
|
||||
// connect log sequence (shape, not values)
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
func TestRegisterToBackend_NoPeersAndSingleEntryPeers_IdenticalLogSequence(t *testing.T) {
|
||||
logs := captureLog(t)
|
||||
|
||||
cfgNoPeers := &BackendConfig{
|
||||
Address: "127.0.0.1",
|
||||
Port: testPortBase + 20,
|
||||
From: "xce_apiserver_test",
|
||||
Key: "keepsecret$",
|
||||
}
|
||||
if err := RegisterToBackend(cfgNoPeers); err == nil {
|
||||
t.Fatal("expected RegisterToBackend(no peers) to fail against a closed port")
|
||||
}
|
||||
noPeersMessages := logMessages(logs) // drains the observer buffer
|
||||
|
||||
cfgOnePeer := &BackendConfig{
|
||||
Peers: []string{"127.0.0.1:28601"},
|
||||
From: "xce_apiserver_test",
|
||||
Key: "keepsecret$",
|
||||
}
|
||||
if err := RegisterToBackend(cfgOnePeer); err == nil {
|
||||
t.Fatal("expected RegisterToBackend(single-entry peers) to fail against a closed port")
|
||||
}
|
||||
onePeerMessages := logMessages(logs)
|
||||
|
||||
if len(noPeersMessages) == 0 {
|
||||
t.Fatal("expected at least one log message from RegisterToBackend(no peers)")
|
||||
}
|
||||
if !equalStringSlices(noPeersMessages, onePeerMessages) {
|
||||
t.Errorf("log message sequence diverges between no-peers and "+
|
||||
"single-entry-peers:\n no-peers: %v\n single-entry: %v",
|
||||
noPeersMessages, onePeerMessages)
|
||||
}
|
||||
}
|
||||
|
||||
func logMessages(logs *observer.ObservedLogs) []string {
|
||||
entries := logs.TakeAll()
|
||||
msgs := make([]string, len(entries))
|
||||
for i, e := range entries {
|
||||
msgs[i] = e.Message
|
||||
}
|
||||
return msgs
|
||||
}
|
||||
|
||||
func equalStringSlices(a, b []string) bool {
|
||||
if len(a) != len(b) {
|
||||
return false
|
||||
}
|
||||
for i := range a {
|
||||
if a[i] != b[i] {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// Structural regression guard: source config structs are package-level
|
||||
// (source-text scan of the real, current on-disk clib.go -- same genre as
|
||||
// scheduler's/worker's structural regression tests for the identical
|
||||
// use-after-scope hazard).
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
func TestClibSource_ClusterAndClientConfigsArePackageLevel(t *testing.T) {
|
||||
src, err := os.ReadFile("clib.go")
|
||||
if err != nil {
|
||||
t.Fatalf("could not read clib.go source for scan: %v", err)
|
||||
}
|
||||
text := string(src)
|
||||
|
||||
if !strings.Contains(text, "var gClusterCfg C.xce_cluster_config_t") {
|
||||
t.Error("gClusterCfg must be declared as a package-level var " +
|
||||
"(`var gClusterCfg C.xce_cluster_config_t`) -- a function-local " +
|
||||
"variable would go out of scope while xce_cluster_config_to_client_config()'s " +
|
||||
"aliased pointers (and xcecli_ctx_init()'s retained ctx->conf pointer) " +
|
||||
"are still in use, a use-after-scope bug (see tasks.md's Implementation " +
|
||||
"Notes for task 2.1)")
|
||||
}
|
||||
if !strings.Contains(text, "var gClientCfg C.xce_client_config_t") {
|
||||
t.Error("gClientCfg must be declared as a package-level var " +
|
||||
"(`var gClientCfg C.xce_client_config_t`) -- xcecli_ctx_init() " +
|
||||
"stores a raw pointer to it (ctx->conf) dereferenced for the ctx's " +
|
||||
"entire runtime life, not only the initial connect")
|
||||
}
|
||||
|
||||
// Exactly one xce_backend_init() call site: the former per-peer loop
|
||||
// called it once per candidate (a real loop, not a single call), and the
|
||||
// former no-Peers branch duplicated it a second time in the same
|
||||
// function. A single call site proves both are gone.
|
||||
first := strings.Index(text, "C.xce_backend_init(")
|
||||
if first < 0 {
|
||||
t.Fatal("C.xce_backend_init(...) call site not found in clib.go")
|
||||
}
|
||||
if strings.Index(text[first+1:], "C.xce_backend_init(") >= 0 {
|
||||
t.Error("clib.go must contain exactly ONE C.xce_backend_init() call site -- " +
|
||||
"more than one means the deleted per-peer loop or the separate " +
|
||||
"no-Peers branch is still present")
|
||||
}
|
||||
|
||||
if strings.Contains(text, "func tryConnect(") {
|
||||
t.Error("tryConnect() must be deleted entirely -- its per-call " +
|
||||
"C.CString(host)/C.CString(from)/C.CString(key) + deferred C.free " +
|
||||
"immediately after a single xce_backend_init() call left ctx->conf's " +
|
||||
"aliased masters[]/from/key pointers dangling for the ctx's remaining " +
|
||||
"runtime life (see status report CONCERNS for the precise finding)")
|
||||
}
|
||||
}
|
||||
|
|
@ -8,10 +8,13 @@ type BackendConfig struct {
|
|||
From string // identity sent to controller (default: "xce_apiserver")
|
||||
Key string // authentication key
|
||||
|
||||
// Peers lists additional controller candidates for HA mode in "host:port"
|
||||
// format. When non-empty, RegisterToBackend tries each candidate in order
|
||||
// and connects to whichever accepts the join as primary. Address+Port
|
||||
// serve as a single-host fallback when Peers is empty.
|
||||
// Peers lists additional controller candidates for HA mode, each entry in
|
||||
// "[name=]host:port" format -- the same syntax scheduler's and worker's
|
||||
// --peers flag already accept (Req 6.1), parsed via the shared C parser
|
||||
// xce_parse_peers (candidates.go). When non-empty, RegisterToBackend
|
||||
// tries each candidate in order and connects to whichever accepts the
|
||||
// join as primary. Address+Port serve as a single-host fallback when
|
||||
// Peers is empty.
|
||||
Peers []string
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -57,6 +57,7 @@ func cJobArrayToModels(jobArray *C.xce_job_submit_t) []*models.JobInfo {
|
|||
resReq: goString(s.resreq),
|
||||
minTasks: int(s.mintasks),
|
||||
maxTasks: int(s.maxtasks),
|
||||
fromHost: goString(s.from_host),
|
||||
}
|
||||
if s.start_info != nil {
|
||||
si := s.start_info
|
||||
|
|
@ -97,6 +98,7 @@ func cJobToModel(job *C.xce_job_t) *models.JobInfo {
|
|||
resReq: goString(s.resreq),
|
||||
minTasks: int(s.mintasks),
|
||||
maxTasks: int(s.maxtasks),
|
||||
fromHost: goString(s.from_host),
|
||||
}
|
||||
if s.start_info != nil {
|
||||
si := s.start_info
|
||||
|
|
@ -311,6 +313,9 @@ func modelJobToSubmit(job *models.JobInfo) (*C.xce_job_submit_t, *C.job_submit_d
|
|||
if job.ResReq != "" {
|
||||
sd.resreq = C.CString(job.ResReq)
|
||||
}
|
||||
if job.FromHost != "" {
|
||||
sd.from_host = C.CString(job.FromHost)
|
||||
}
|
||||
sd.mintasks = C.uint(job.NumProcessors)
|
||||
if sd.mintasks == 0 {
|
||||
sd.mintasks = 1
|
||||
|
|
@ -347,6 +352,7 @@ func freeJobSubmit(js *C.xce_job_submit_t, sd *C.job_submit_data_t, si *C.job_st
|
|||
C.free(unsafe.Pointer(sd.user_group))
|
||||
C.free(unsafe.Pointer(sd.job_description))
|
||||
C.free(unsafe.Pointer(sd.resreq))
|
||||
C.free(unsafe.Pointer(sd.from_host))
|
||||
C.free(unsafe.Pointer(sd))
|
||||
}
|
||||
if js != nil {
|
||||
|
|
|
|||
|
|
@ -157,6 +157,7 @@ type rawSubmitData struct {
|
|||
minTasks int
|
||||
maxTasks int
|
||||
startInfo *rawStartInfo
|
||||
fromHost string
|
||||
}
|
||||
|
||||
// rawHostReason mirrors the C xce_host_reason_t (one host-level PEND reason).
|
||||
|
|
@ -283,6 +284,7 @@ func rawJobToModel(r rawJobData) *models.JobInfo {
|
|||
info.ResReq = s.resReq
|
||||
info.NumProcessors = s.minTasks
|
||||
info.MaxNumProcessors = s.maxTasks
|
||||
info.FromHost = s.fromHost
|
||||
if s.startInfo != nil {
|
||||
si := s.startInfo
|
||||
info.Command = si.cmdline
|
||||
|
|
|
|||
|
|
@ -44,6 +44,7 @@ func TestRawJobToModel_BasicFields(t *testing.T) {
|
|||
jobName: "myjob",
|
||||
minTasks: 1,
|
||||
maxTasks: 4,
|
||||
fromHost: "203.0.113.7",
|
||||
startInfo: &rawStartInfo{
|
||||
cmdline: "sleep 60",
|
||||
},
|
||||
|
|
@ -82,6 +83,25 @@ func TestRawJobToModel_BasicFields(t *testing.T) {
|
|||
if info.RunTime != 42 {
|
||||
t.Errorf("RunTime: got %d, want 42", info.RunTime)
|
||||
}
|
||||
if info.FromHost != "203.0.113.7" {
|
||||
t.Errorf("FromHost: got %q, want %q", info.FromHost, "203.0.113.7")
|
||||
}
|
||||
}
|
||||
|
||||
// bug-1676: FromHost must round-trip through rawSubmitData into JobInfo like
|
||||
// every other submit-time field, and must be empty-safe (no submit data, or
|
||||
// submit data with no from_host, must not panic and must leave FromHost "").
|
||||
func TestRawJobToModel_FromHost_EmptySafe(t *testing.T) {
|
||||
r := rawJobData{
|
||||
jobID: 1,
|
||||
submit: &rawSubmitData{queue: "normal"},
|
||||
}
|
||||
|
||||
info := rawJobToModel(r)
|
||||
|
||||
if info.FromHost != "" {
|
||||
t.Errorf("FromHost: got %q, want empty string when submit.fromHost is unset", info.FromHost)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRawJobToModel_NilSubmit(t *testing.T) {
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import (
|
|||
"apiserver/models"
|
||||
"apiserver/store"
|
||||
logger "apiserver/utils"
|
||||
"sync"
|
||||
|
||||
"go.uber.org/zap"
|
||||
)
|
||||
|
|
@ -11,6 +12,66 @@ import (
|
|||
// DefaultHandler implements NotificationHandler by writing to stores.
|
||||
type DefaultHandler struct{}
|
||||
|
||||
// defaultHostConfigName is the cluster-wide config sentinel in hosts.yaml
|
||||
// (never a real worker) whose fields worker.c's apply_join_state()/
|
||||
// find_matched_hostconfig() already falls back to on the C side for any
|
||||
// worker with no host-specific hosts.yaml entry of its own.
|
||||
const defaultHostConfigName = "default"
|
||||
|
||||
// bug-1673: a dynamically-joined worker with no hosts.yaml entry under its
|
||||
// own literal hostname never receives its own OnHostConfigUpdated call, so
|
||||
// MaxSlot/HostModel/HostType/DisptWindow stayed at Go's zero-value forever
|
||||
// -- decoupled from the C-side scheduling path, which already resolves the
|
||||
// real value via the "default" wildcard entry. These package-level vars
|
||||
// cache that entry's fields plus which hosts have their OWN explicit
|
||||
// (non-default) config, so OnHostUpdated can apply the same fallback for
|
||||
// display purposes that worker.c already applies for real dispatch.
|
||||
var (
|
||||
hostConfigMu sync.Mutex
|
||||
defaultHostConfig *models.HostInfo
|
||||
hostsWithExplicitConfig = map[string]bool{}
|
||||
)
|
||||
|
||||
// resetHostConfigCache clears the cached "default" pseudo-host config and
|
||||
// explicit-config tracking. Test-only: called from this package's _test.go
|
||||
// store-reset helpers, so bug-1673 fallback state doesn't leak between
|
||||
// tests the way store.InitStores() already prevents for the stores
|
||||
// themselves.
|
||||
func resetHostConfigCache() {
|
||||
hostConfigMu.Lock()
|
||||
defaultHostConfig = nil
|
||||
hostsWithExplicitConfig = map[string]bool{}
|
||||
hostConfigMu.Unlock()
|
||||
}
|
||||
|
||||
// applyDefaultHostConfigFallback copies the cached "default" pseudo-host's
|
||||
// config-sourced fields onto host, if one has been observed yet. No-op
|
||||
// otherwise (nothing to fall back to).
|
||||
//
|
||||
// A negative MaxSlot is HOSTCONFIG_MXJ_AUTO_DETECT (bug-1556): "no explicit
|
||||
// cap, use this host's real CPU count" -- the same resolution worker.c's
|
||||
// apply_join_state()/find_matched_hostconfig() already performs for actual
|
||||
// dispatch ("if (nslots<0) nslots = maxcpus"). Resolving it here too means
|
||||
// this display fallback shows the host's real usable capacity (matching
|
||||
// what dispatch will actually honor) instead of a bare "-1" that would
|
||||
// leave the reporter's original "is ncpus equal slots?" question (gitea
|
||||
// #40) just as unanswered as the "0" it replaces.
|
||||
func applyDefaultHostConfigFallback(host *models.HostInfo) {
|
||||
hostConfigMu.Lock()
|
||||
def := defaultHostConfig
|
||||
hostConfigMu.Unlock()
|
||||
if def == nil {
|
||||
return
|
||||
}
|
||||
host.HostModel = def.HostModel
|
||||
host.HostType = def.HostType
|
||||
host.MaxSlot = def.MaxSlot
|
||||
if host.MaxSlot < 0 && host.NumCPUs > 0 {
|
||||
host.MaxSlot = host.NumCPUs
|
||||
}
|
||||
host.DisptWindow = def.DisptWindow
|
||||
}
|
||||
|
||||
func (h *DefaultHandler) OnJobCreated(job *models.JobInfo) {
|
||||
store.Jobs.Put(uint64(job.Unique_JobID), *job)
|
||||
DefaultBroker.Publish(Event{Type: EventJobCreated, JobID: uint64(job.Unique_JobID), Status: models.JobStateStr(job.JobStatus), Job: job})
|
||||
|
|
@ -46,7 +107,10 @@ func (h *DefaultHandler) OnQueueUpdated(queue *models.QueueInfo) {
|
|||
|
||||
// OnHostUpdated merges worker-sourced fields into an existing host entry.
|
||||
// Config-sourced fields (HostModel, HostType, MaxSlot, DisptWindow) are
|
||||
// preserved from the prior entry if one exists.
|
||||
// preserved from the prior entry if one exists. If this is the host's
|
||||
// first-ever registration (no prior entry), fall back to the cached
|
||||
// "default" pseudo-host's config (bug-1673) rather than leaving those
|
||||
// fields at Go's zero-value -- mirrors the C-side wildcard fallback.
|
||||
func (h *DefaultHandler) OnHostUpdated(incoming *models.HostInfo) {
|
||||
merged := *incoming
|
||||
if existing, ok := store.Hosts.Get(merged.HostName); ok {
|
||||
|
|
@ -54,6 +118,8 @@ func (h *DefaultHandler) OnHostUpdated(incoming *models.HostInfo) {
|
|||
merged.HostType = existing.HostType
|
||||
merged.MaxSlot = existing.MaxSlot
|
||||
merged.DisptWindow = existing.DisptWindow
|
||||
} else {
|
||||
applyDefaultHostConfigFallback(&merged)
|
||||
}
|
||||
store.Hosts.Put(merged.HostName, merged)
|
||||
}
|
||||
|
|
@ -61,7 +127,35 @@ func (h *DefaultHandler) OnHostUpdated(incoming *models.HostInfo) {
|
|||
// OnHostConfigUpdated applies config-sourced fields onto an existing worker-registered host.
|
||||
// Hosts not yet registered by a worker are skipped — config entries like "default" (which sets
|
||||
// cluster-wide defaults) or hosts whose daemons haven't started must not appear in the host list.
|
||||
//
|
||||
// The "default" pseudo-host entry itself is cached (bug-1673) rather than
|
||||
// simply discarded, then retroactively backfilled onto any already-
|
||||
// registered host with no explicit config of its own -- "default" can
|
||||
// arrive either before or after a given worker's own OnHostUpdated call.
|
||||
func (h *DefaultHandler) OnHostConfigUpdated(incoming *models.HostInfo) {
|
||||
if incoming.HostName == defaultHostConfigName {
|
||||
hostConfigMu.Lock()
|
||||
cp := *incoming
|
||||
defaultHostConfig = &cp
|
||||
hostConfigMu.Unlock()
|
||||
|
||||
store.Hosts.Range(func(name string, host models.HostInfo) bool {
|
||||
hostConfigMu.Lock()
|
||||
explicit := hostsWithExplicitConfig[name]
|
||||
hostConfigMu.Unlock()
|
||||
if !explicit {
|
||||
applyDefaultHostConfigFallback(&host)
|
||||
store.Hosts.Put(name, host)
|
||||
}
|
||||
return true
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
hostConfigMu.Lock()
|
||||
hostsWithExplicitConfig[incoming.HostName] = true
|
||||
hostConfigMu.Unlock()
|
||||
|
||||
existing, ok := store.Hosts.Get(incoming.HostName)
|
||||
if !ok {
|
||||
return
|
||||
|
|
@ -96,6 +190,36 @@ func (h *DefaultHandler) OnResourceUpdated(res *models.ResourceDefinitionInfo) {
|
|||
func (h *DefaultHandler) OnResourceLocationUpdated(res *models.ResourceDistributionInfo) {
|
||||
key := res.ResourceName + ":" + res.Location
|
||||
store.ResourceLocations.Put(key, *res)
|
||||
|
||||
// resource-value-verification task 4.2 (Requirements 5.1, 5.2, 5.4): trace
|
||||
// the stored value at its point of application, mirroring the scheduler's
|
||||
// sibling trace log added in task 4.1 (core_events.c). One typed "value"
|
||||
// field carrying whichever of StringValue/NumericValue/BooleanValue is
|
||||
// live for res.ValueType, so a downstream grep sees exactly what was
|
||||
// stored, not every possibly-zero field on the struct.
|
||||
//
|
||||
// DO NOT change the "resource-location stored" message string, or the
|
||||
// resource/host/value field names below, without also updating
|
||||
// query_apiserver_resvals() in
|
||||
// .claude/skills/xce-test-resources/run-resource-verify.sh (design.md's
|
||||
// own Revalidation Triggers list this exact pair) -- that script greps
|
||||
// this exact message and parses zap's console-encoder trailing JSON
|
||||
// block for these field names; an unrelated logging cleanup here
|
||||
// silently turns every future verification run into a false
|
||||
// UNREACHABLE/timeout, indistinguishable from a real regression.
|
||||
var valueField zap.Field
|
||||
switch res.ValueType {
|
||||
case int(models.STRING):
|
||||
valueField = zap.String("value", res.StringValue)
|
||||
case int(models.BOOLEAN):
|
||||
valueField = zap.Bool("value", res.BooleanValue)
|
||||
default:
|
||||
valueField = zap.Float64("value", res.NumericValue)
|
||||
}
|
||||
logger.Debug("resource-location stored",
|
||||
zap.String("resource", res.ResourceName),
|
||||
zap.String("host", res.Location),
|
||||
valueField)
|
||||
}
|
||||
|
||||
// dispatchJobArray delivers a locally-expanded compact array push to the
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import (
|
|||
// resetStores reinitializes all global stores for test isolation.
|
||||
func resetStores() {
|
||||
store.InitStores()
|
||||
resetHostConfigCache()
|
||||
}
|
||||
|
||||
// freshBroker swaps DefaultBroker for an isolated instance, restoring the
|
||||
|
|
@ -159,6 +160,148 @@ func TestOnHostConfigUpdated_PartialMerge(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// TestOnHostConfigUpdated_DefaultFallback_ConfigBeforeWorker is a regression
|
||||
// guard for bug-1673 (gitea #40): a dynamically-joined worker (e.g.
|
||||
// "worker-1") with no hosts.yaml entry under its own literal hostname never
|
||||
// receives its own OnHostConfigUpdated call, so MaxSlot/HostModel/HostType
|
||||
// stayed at Go's zero-value forever even though the C-side scheduling path
|
||||
// already resolves the real value via the "default" wildcard entry. This
|
||||
// covers the "default" config arriving BEFORE the worker registers.
|
||||
func TestOnHostConfigUpdated_DefaultFallback_ConfigBeforeWorker(t *testing.T) {
|
||||
resetStores()
|
||||
h := DefaultHandler{}
|
||||
|
||||
// "default" pseudo-host config push arrives first (cluster startup).
|
||||
h.OnHostConfigUpdated(&models.HostInfo{
|
||||
HostName: defaultHostConfigName,
|
||||
HostModel: "generic",
|
||||
HostType: "x86_64-linux-gnu",
|
||||
MaxSlot: -1, // HOSTCONFIG_MXJ_AUTO_DETECT sentinel (bug-1556)
|
||||
})
|
||||
|
||||
// Dynamically-joined worker registers under its own name, never listed
|
||||
// in hosts.yaml -- so no matching OnHostConfigUpdated call ever fires
|
||||
// for "worker-1" itself.
|
||||
h.OnHostUpdated(&models.HostInfo{
|
||||
HostName: "worker-1",
|
||||
NumCPUs: 22,
|
||||
})
|
||||
|
||||
got, ok := store.Hosts.Get("worker-1")
|
||||
if !ok {
|
||||
t.Fatal("worker-1 should exist after OnHostUpdated")
|
||||
}
|
||||
if got.MaxSlot != 22 {
|
||||
t.Errorf("MaxSlot: got %d, want 22 (fallen back to \"default\", -1 auto-detect resolved to NumCPUs, not stuck at 0)", got.MaxSlot)
|
||||
}
|
||||
if got.HostModel != "generic" {
|
||||
t.Errorf("HostModel: got %q, want %q (fallen back to \"default\")", got.HostModel, "generic")
|
||||
}
|
||||
if got.HostType != "x86_64-linux-gnu" {
|
||||
t.Errorf("HostType: got %q, want %q (fallen back to \"default\")", got.HostType, "x86_64-linux-gnu")
|
||||
}
|
||||
// Worker-sourced field preserved throughout.
|
||||
if got.NumCPUs != 22 {
|
||||
t.Errorf("NumCPUs: got %d, want 22 (should be preserved)", got.NumCPUs)
|
||||
}
|
||||
}
|
||||
|
||||
// TestOnHostConfigUpdated_DefaultFallback_WorkerBeforeConfig covers the
|
||||
// opposite ordering: the worker registers first, and "default" arrives
|
||||
// later. The fallback must be retroactively backfilled onto the
|
||||
// already-registered host.
|
||||
func TestOnHostConfigUpdated_DefaultFallback_WorkerBeforeConfig(t *testing.T) {
|
||||
resetStores()
|
||||
h := DefaultHandler{}
|
||||
|
||||
h.OnHostUpdated(&models.HostInfo{
|
||||
HostName: "worker-1",
|
||||
NumCPUs: 16,
|
||||
})
|
||||
|
||||
got, ok := store.Hosts.Get("worker-1")
|
||||
if !ok || got.MaxSlot != 0 {
|
||||
t.Fatalf("precondition: worker-1 should exist with MaxSlot still 0 before \"default\" arrives, got ok=%v MaxSlot=%d", ok, got.MaxSlot)
|
||||
}
|
||||
|
||||
h.OnHostConfigUpdated(&models.HostInfo{
|
||||
HostName: defaultHostConfigName,
|
||||
HostModel: "generic",
|
||||
HostType: "x86_64-linux-gnu",
|
||||
MaxSlot: -1,
|
||||
})
|
||||
|
||||
got, ok = store.Hosts.Get("worker-1")
|
||||
if !ok {
|
||||
t.Fatal("worker-1 should still exist after \"default\" backfill")
|
||||
}
|
||||
if got.MaxSlot != 16 {
|
||||
t.Errorf("MaxSlot: got %d, want 16 (retroactively backfilled from \"default\", -1 auto-detect resolved to NumCPUs)", got.MaxSlot)
|
||||
}
|
||||
if got.NumCPUs != 16 {
|
||||
t.Errorf("NumCPUs: got %d, want 16 (should be preserved)", got.NumCPUs)
|
||||
}
|
||||
}
|
||||
|
||||
// TestOnHostConfigUpdated_DefaultFallback_UnresolvableStaysNegative covers
|
||||
// the edge case where NumCPUs isn't known yet (0) -- there's nothing to
|
||||
// resolve the -1 auto-detect sentinel to, so it's left as-is rather than
|
||||
// silently becoming a misleading MaxSlot of 0.
|
||||
func TestOnHostConfigUpdated_DefaultFallback_UnresolvableStaysNegative(t *testing.T) {
|
||||
resetStores()
|
||||
h := DefaultHandler{}
|
||||
|
||||
h.OnHostConfigUpdated(&models.HostInfo{
|
||||
HostName: defaultHostConfigName,
|
||||
MaxSlot: -1,
|
||||
})
|
||||
h.OnHostUpdated(&models.HostInfo{HostName: "worker-1"}) // NumCPUs unset (0)
|
||||
|
||||
got, ok := store.Hosts.Get("worker-1")
|
||||
if !ok {
|
||||
t.Fatal("worker-1 should exist after OnHostUpdated")
|
||||
}
|
||||
if got.MaxSlot != -1 {
|
||||
t.Errorf("MaxSlot: got %d, want -1 (unresolvable with NumCPUs=0, must not become a misleading 0)", got.MaxSlot)
|
||||
}
|
||||
}
|
||||
|
||||
// TestOnHostConfigUpdated_DefaultFallback_DoesNotOverrideExplicitConfig
|
||||
// proves the "default" fallback never clobbers a host's own real,
|
||||
// host-specific hosts.yaml entry -- whichever order the two config pushes
|
||||
// and the worker registration arrive in.
|
||||
func TestOnHostConfigUpdated_DefaultFallback_DoesNotOverrideExplicitConfig(t *testing.T) {
|
||||
resetStores()
|
||||
h := DefaultHandler{}
|
||||
|
||||
h.OnHostUpdated(&models.HostInfo{HostName: "node01", NumCPUs: 8})
|
||||
h.OnHostConfigUpdated(&models.HostInfo{
|
||||
HostName: "node01",
|
||||
HostModel: "modelA",
|
||||
HostType: "CENTOS7",
|
||||
MaxSlot: 16,
|
||||
})
|
||||
|
||||
// "default" arrives after -- must not overwrite node01's real config.
|
||||
h.OnHostConfigUpdated(&models.HostInfo{
|
||||
HostName: defaultHostConfigName,
|
||||
HostModel: "generic",
|
||||
HostType: "x86_64-linux-gnu",
|
||||
MaxSlot: -1,
|
||||
})
|
||||
|
||||
got, ok := store.Hosts.Get("node01")
|
||||
if !ok {
|
||||
t.Fatal("node01 should exist")
|
||||
}
|
||||
if got.MaxSlot != 16 {
|
||||
t.Errorf("MaxSlot: got %d, want 16 (explicit config must not be overridden by \"default\")", got.MaxSlot)
|
||||
}
|
||||
if got.HostModel != "modelA" {
|
||||
t.Errorf("HostModel: got %q, want %q (explicit config must not be overridden by \"default\")", got.HostModel, "modelA")
|
||||
}
|
||||
}
|
||||
|
||||
func TestOnClusterUpdated_NewCluster(t *testing.T) {
|
||||
resetStores()
|
||||
h := DefaultHandler{}
|
||||
|
|
|
|||
|
|
@ -1,24 +1,80 @@
|
|||
package backend
|
||||
|
||||
import "sync/atomic"
|
||||
import (
|
||||
"sync/atomic"
|
||||
"time"
|
||||
)
|
||||
|
||||
// backendDegraded is the fail-fast circuit breaker (component-failover-adoption
|
||||
// Req 4): when the controller connection is down, backend-requiring API
|
||||
// requests return a clear "backend unavailable" error immediately instead of
|
||||
// hanging until the connection is restored.
|
||||
// health.go implements ApiserverBackendHealth (component-failover-adoption
|
||||
// task 4.5, boundary: ApiserverBackendHealth, Req 4.1-4.2): a small,
|
||||
// independent circuit breaker that lets apiserver's HTTP request handlers
|
||||
// fail fast with a clear "backend unavailable" error instead of blocking on
|
||||
// the client library's own retry budget (XCECLI_FAILOVER_CALL:
|
||||
// max_retries=20, backoff capped at 5000ms -- roughly 1-2 minutes total,
|
||||
// client_failover.c:218,220) when the backend connection is known-degraded.
|
||||
// clib.go's shared callBackend helper is the only caller of IsDegraded()/
|
||||
// MarkDegraded() (Req 4.1); calls that never touch gCtx (e.g. GenerateJobID)
|
||||
// don't route through callBackend and are therefore unaffected (Req 4.2).
|
||||
//
|
||||
// Lifecycle:
|
||||
// - tripped by a write-path transport failure (callBackend) and by a failed
|
||||
// heartbeat tick (StartHeartbeat);
|
||||
// - cleared by a successful heartbeat recovery (StartHeartbeat → rejoinBackend
|
||||
// → MarkHealthy), which also re-joins the apiserver session so pushes resume.
|
||||
var backendDegraded atomic.Bool
|
||||
// State model: a single atomic "degraded-until" unix-nano timestamp
|
||||
// (int64 via sync/atomic), the same pattern heartbeat.go's
|
||||
// heartbeatIntervalSecs already uses for a lock-free, concurrency-safe
|
||||
// package-level value. IsDegraded() is true while time.Now() is before that
|
||||
// timestamp -- process-local, in-memory only, reset on restart (matches the
|
||||
// rest of apiserver's in-memory state model).
|
||||
//
|
||||
// Deliberately no build tag: pure Go, no CGO involved in this file at all --
|
||||
// it compiles and behaves identically under CGO_ENABLED=0 and =1, so this
|
||||
// state and its tests need neither cgo nor a live backend connection.
|
||||
//
|
||||
// Deliberately independent of reconnect.go's needsRejoin flag (Req 2 AC5,
|
||||
// a design-validation third-pass correction). This state's TTL-based
|
||||
// auto-clear exists purely so a request handler eventually probes the
|
||||
// backend again even without an explicit MarkHealthy() call -- a
|
||||
// legitimate circuit-breaker half-open pattern. It must NEVER be read by,
|
||||
// or used to gate, the rejoin-retry logic in reconnect.go/heartbeat.go:
|
||||
// needsRejoin tracks "have we actually re-joined the cluster yet" and
|
||||
// clears only on a genuine xcecli_join() success, with no timeout of its
|
||||
// own. If this state's TTL expired before an actual rejoin succeeded,
|
||||
// gating the rejoin retry on "still degraded" would silently stop retrying
|
||||
// the join even though apiserver is still not actually a cluster member --
|
||||
// exactly the class of bug an earlier draft of the heartbeat-idle-path fix
|
||||
// (task 4.4) introduced and design validation's third pass corrected. See
|
||||
// reconnect.go's checkAndPerformRejoin, which this file's MarkHealthy() is
|
||||
// called FROM (at the moment a pending rejoin's xcecli_join() call
|
||||
// succeeds) but which never calls back into IsDegraded() itself.
|
||||
|
||||
// IsDegraded reports whether the backend controller is currently unreachable.
|
||||
func IsDegraded() bool { return backendDegraded.Load() }
|
||||
// degradedUntilNanos holds the unix-nano timestamp before which IsDegraded()
|
||||
// reports true. Its zero value (0, i.e. the Unix epoch) means "never marked
|
||||
// degraded" -- correctly reports healthy at process start, since time.Now()
|
||||
// is always after the epoch.
|
||||
var degradedUntilNanos int64
|
||||
|
||||
// MarkDegraded trips the circuit breaker — backend calls now fail fast.
|
||||
func MarkDegraded() { backendDegraded.Store(true) }
|
||||
// IsDegraded reports whether the backend connection is currently considered
|
||||
// degraded: true while time.Now() is before the timestamp MarkDegraded most
|
||||
// recently set. Called by clib.go's shared callBackend helper as the
|
||||
// pre-call fail-fast check before every gCtx.ctx-based backend call
|
||||
// (Req 4.1).
|
||||
func IsDegraded() bool {
|
||||
return time.Now().UnixNano() < atomic.LoadInt64(°radedUntilNanos)
|
||||
}
|
||||
|
||||
// MarkHealthy clears the circuit breaker after a successful reconnect/rejoin.
|
||||
func MarkHealthy() { backendDegraded.Store(false) }
|
||||
// MarkDegraded opens the circuit for ttl: IsDegraded() reports true until
|
||||
// ttl elapses or MarkHealthy() is called first, whichever happens sooner
|
||||
// (a half-open circuit breaker -- the TTL alone lets a healthy backend get
|
||||
// probed again even if the explicit-clear signal never fires). Called by
|
||||
// clib.go's callBackend when a backend call's own outcome indicates its
|
||||
// retry budget was exhausted (-E_XCE_NOT_READY).
|
||||
func MarkDegraded(ttl time.Duration) {
|
||||
atomic.StoreInt64(°radedUntilNanos, time.Now().Add(ttl).UnixNano())
|
||||
}
|
||||
|
||||
// MarkHealthy clears the degraded state immediately, regardless of any
|
||||
// pending TTL. Called by reconnect.go's checkAndPerformRejoin at the exact
|
||||
// point a pending rejoin's xcecli_join() call succeeds -- a single call
|
||||
// site reached from both the write-path (task 4.3) and the idle-period
|
||||
// heartbeat path (task 4.4), since both already funnel through
|
||||
// checkAndPerformRejoin.
|
||||
func MarkHealthy() {
|
||||
atomic.StoreInt64(°radedUntilNanos, 0)
|
||||
}
|
||||
|
|
|
|||
283
src/frontend/apiserver/backend/health_rejoin_test.go
Normal file
283
src/frontend/apiserver/backend/health_rejoin_test.go
Normal file
|
|
@ -0,0 +1,283 @@
|
|||
//go:build cgo
|
||||
|
||||
package backend
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// health_rejoin_test.go exercises two things task 4.5 adds
|
||||
// (component-failover-adoption, boundary: ApiserverBackendBridge/
|
||||
// ApiserverBackendHealth, Req 4.1-4.2):
|
||||
//
|
||||
// 1. clib.go's callBackend shared helper: the pre-call IsDegraded()
|
||||
// fail-fast check and the post-call retry-budget-exhaustion ->
|
||||
// MarkDegraded() transition.
|
||||
// 2. The independence guarantee between health.go's degraded-until state
|
||||
// and reconnect.go's needsRejoin/checkAndPerformRejoin -- the task's
|
||||
// own critical constraint (design.md's "Deliberately independent of
|
||||
// needs_rejoin" correction): IsDegraded() must never gate whether a
|
||||
// rejoin gets retried.
|
||||
//
|
||||
// Kept //go:build cgo since callBackend/checkAndPerformRejoin/needsRejoin
|
||||
// only exist in the CGO build -- this file itself does not `import "C"` (a
|
||||
// _test.go file can never do that directly, golang/go#13938): callBackend's
|
||||
// own signature (func(string, func() int) (int, error)) is plain Go, the
|
||||
// same test seam pattern reconnect.go's performRejoinFn already
|
||||
// established, so this file can drive it directly with a scripted closure
|
||||
// -- no live backend connection or initialized gCtx required at all.
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// Req 4.1 -- callBackend's pre-call fail-fast check
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
func TestCallBackend_Degraded_SkipsUnderlyingCallAndReturnsUnavailable(t *testing.T) {
|
||||
resetHealthForTest()
|
||||
defer resetHealthForTest()
|
||||
MarkDegraded(1 * time.Minute)
|
||||
|
||||
calls := 0
|
||||
rc, err := callBackend("xcecli_test", func() int {
|
||||
calls++
|
||||
return 0
|
||||
})
|
||||
|
||||
if calls != 0 {
|
||||
t.Errorf("expected the underlying call to be skipped entirely while "+
|
||||
"degraded, but it was invoked %d time(s)", calls)
|
||||
}
|
||||
if err != ErrBackendUnavailable {
|
||||
t.Errorf("expected ErrBackendUnavailable, got %v", err)
|
||||
}
|
||||
if rc != 0 {
|
||||
t.Errorf("expected rc=0 on the fail-fast path, got %d", rc)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCallBackend_Healthy_InvokesUnderlyingCallAndReturnsItsResult(t *testing.T) {
|
||||
resetHealthForTest()
|
||||
defer resetHealthForTest()
|
||||
|
||||
calls := 0
|
||||
rc, err := callBackend("xcecli_test", func() int {
|
||||
calls++
|
||||
return 7
|
||||
})
|
||||
|
||||
if calls != 1 {
|
||||
t.Errorf("expected exactly 1 underlying call while healthy, got %d", calls)
|
||||
}
|
||||
if err != nil {
|
||||
t.Errorf("expected no error for a non-negative rc, got %v", err)
|
||||
}
|
||||
if rc != 7 {
|
||||
t.Errorf("expected callBackend to return the underlying rc (7), got %d", rc)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCallBackend_NegativeRc_ReturnsErrorNamingTheCall(t *testing.T) {
|
||||
resetHealthForTest()
|
||||
defer resetHealthForTest()
|
||||
|
||||
_, err := callBackend("xcecli_test", func() int {
|
||||
return -5
|
||||
})
|
||||
|
||||
if err == nil {
|
||||
t.Fatal("expected an error for a negative rc")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "xcecli_test") || !strings.Contains(err.Error(), "-5") {
|
||||
t.Errorf("expected error to name the call and rc, got %q", err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// Req 4.1 -- retry-budget exhaustion (-E_XCE_NOT_READY) marks degraded
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
func TestCallBackend_RetryBudgetExhausted_MarksDegraded(t *testing.T) {
|
||||
resetHealthForTest()
|
||||
defer resetHealthForTest()
|
||||
|
||||
_, err := callBackend("xcecli_test", func() int {
|
||||
return notReadyRC
|
||||
})
|
||||
|
||||
if err == nil {
|
||||
t.Fatal("expected an error for -E_XCE_NOT_READY")
|
||||
}
|
||||
if !IsDegraded() {
|
||||
t.Error("expected IsDegraded() to be true after a call returns -E_XCE_NOT_READY")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCallBackend_OtherNegativeRc_DoesNotMarkDegraded(t *testing.T) {
|
||||
resetHealthForTest()
|
||||
defer resetHealthForTest()
|
||||
|
||||
// A generic failure (e.g. -EINVAL) is not the same signal as
|
||||
// retry-budget exhaustion -- only -E_XCE_NOT_READY should open the
|
||||
// circuit.
|
||||
_, err := callBackend("xcecli_test", func() int {
|
||||
return -22 // -EINVAL, unrelated to retry-budget exhaustion
|
||||
})
|
||||
|
||||
if err == nil {
|
||||
t.Fatal("expected an error for a negative rc")
|
||||
}
|
||||
if IsDegraded() {
|
||||
t.Error("expected IsDegraded() to remain false for a non--E_XCE_NOT_READY failure")
|
||||
}
|
||||
}
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// Structural regression guard: all wrapper functions route through the
|
||||
// single shared callBackend helper, not a duplicated per-wrapper copy.
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
func TestClibSource_BackendCallWrappersCallSharedCallBackendHelper(t *testing.T) {
|
||||
clibSrc := readSourceForTest(t, "clib.go")
|
||||
|
||||
count := strings.Count(clibSrc, "callBackend(")
|
||||
// 5 real call sites (SubmitJob/ControlJob/ControlQueue/ControlHost/
|
||||
// FetchFile) + 1 for callBackend's own func declaration line.
|
||||
if count < 6 {
|
||||
t.Errorf("expected at least 6 occurrences of callBackend( in clib.go "+
|
||||
"(the function declaration plus all 5 wrapper call sites), got %d -- "+
|
||||
"the shared pre/post-call helper must not be duplicated or skipped "+
|
||||
"per wrapper", count)
|
||||
}
|
||||
}
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// Critical constraint -- IsDegraded() and the rejoin-retry mechanism
|
||||
// (needsRejoin/checkAndPerformRejoin) are genuinely independent state
|
||||
// machines. This is the design's own explicitly-called-out regression
|
||||
// guard: an earlier draft of task 4.4's fix gated the heartbeat-idle-path
|
||||
// rejoin retry on "still degraded," which would let this state's TTL
|
||||
// silently stop retrying a join that never actually succeeded.
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
func TestCheckAndPerformRejoin_FiresAndSucceedsWhileIsDegradedIsTrue(t *testing.T) {
|
||||
captureLog(t)
|
||||
resetNeedsRejoinForTest()
|
||||
resetHealthForTest()
|
||||
defer resetHealthForTest()
|
||||
|
||||
// Force the circuit breaker open -- if checkAndPerformRejoin were
|
||||
// (incorrectly) gated on IsDegraded(), this would suppress the retry.
|
||||
MarkDegraded(1 * time.Minute)
|
||||
if !IsDegraded() {
|
||||
t.Fatal("setup: expected IsDegraded() to be true")
|
||||
}
|
||||
|
||||
atomic.StoreInt32(&needsRejoin, 1)
|
||||
orig := performRejoinFn
|
||||
defer func() { performRejoinFn = orig }()
|
||||
|
||||
called := false
|
||||
performRejoinFn = func() int {
|
||||
called = true
|
||||
return 0 // success
|
||||
}
|
||||
|
||||
checkAndPerformRejoin()
|
||||
|
||||
if !called {
|
||||
t.Error("expected checkAndPerformRejoin to invoke performRejoinFn even " +
|
||||
"while IsDegraded() is true -- the rejoin-retry mechanism must be " +
|
||||
"completely independent of the circuit-breaker's degraded state")
|
||||
}
|
||||
if atomic.LoadInt32(&needsRejoin) != 0 {
|
||||
t.Error("expected needsRejoin to clear on a successful rejoin, " +
|
||||
"regardless of IsDegraded()")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckAndPerformRejoin_SuccessfulRejoin_MarksHealthy(t *testing.T) {
|
||||
captureLog(t)
|
||||
resetNeedsRejoinForTest()
|
||||
resetHealthForTest()
|
||||
defer resetHealthForTest()
|
||||
|
||||
// Start degraded (as would be realistic: a prior call exhausted its
|
||||
// retry budget, which is presumably WHY a rejoin is now pending).
|
||||
MarkDegraded(1 * time.Minute)
|
||||
|
||||
atomic.StoreInt32(&needsRejoin, 1)
|
||||
orig := performRejoinFn
|
||||
defer func() { performRejoinFn = orig }()
|
||||
performRejoinFn = func() int { return 0 }
|
||||
|
||||
checkAndPerformRejoin()
|
||||
|
||||
if IsDegraded() {
|
||||
t.Error("expected a successful rejoin (via checkAndPerformRejoin) to " +
|
||||
"call MarkHealthy() and clear the degraded state")
|
||||
}
|
||||
}
|
||||
|
||||
func TestMarkDegraded_TTLExpiry_DoesNotTriggerOrSuppressRejoin(t *testing.T) {
|
||||
resetNeedsRejoinForTest()
|
||||
resetHealthForTest()
|
||||
defer resetHealthForTest()
|
||||
|
||||
orig := performRejoinFn
|
||||
defer func() { performRejoinFn = orig }()
|
||||
called := false
|
||||
performRejoinFn = func() int {
|
||||
called = true
|
||||
return 0
|
||||
}
|
||||
|
||||
// Mark degraded with a short TTL and let it expire, with NO rejoin
|
||||
// pending (needsRejoin stays 0 throughout) and callAndPerformRejoin
|
||||
// never invoked.
|
||||
MarkDegraded(10 * time.Millisecond)
|
||||
time.Sleep(40 * time.Millisecond)
|
||||
|
||||
if IsDegraded() {
|
||||
t.Fatal("setup: expected the TTL to have expired")
|
||||
}
|
||||
if called {
|
||||
t.Error("expected the degraded-state TTL expiring on its own to never " +
|
||||
"call performRejoinFn -- these two state machines must stay independent")
|
||||
}
|
||||
if atomic.LoadInt32(&needsRejoin) != 0 {
|
||||
t.Error("expected needsRejoin to remain unset -- the TTL expiring has " +
|
||||
"nothing to do with whether a rejoin is pending")
|
||||
}
|
||||
|
||||
// Also confirm the reverse direction directly through checkAndPerformRejoin:
|
||||
// with needsRejoin genuinely 0 (nothing pending) and now-healthy state,
|
||||
// checkAndPerformRejoin is a no-op, as it always was pre-task-4.5.
|
||||
checkAndPerformRejoin()
|
||||
if called {
|
||||
t.Error("expected checkAndPerformRejoin to remain a no-op when " +
|
||||
"needsRejoin is unset, TTL expiry or not")
|
||||
}
|
||||
}
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// Req 4.2 -- calls that don't touch gCtx are genuinely unaffected by the
|
||||
// degraded state (a quick confirmation, not new test infrastructure).
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
func TestGenerateJobID_UnaffectedByDegradedState(t *testing.T) {
|
||||
resetHealthForTest()
|
||||
defer resetHealthForTest()
|
||||
MarkDegraded(1 * time.Minute)
|
||||
|
||||
id, err := GenerateJobID(1, 1, 1)
|
||||
|
||||
if err != nil {
|
||||
t.Errorf("expected GenerateJobID to succeed while degraded (it never "+
|
||||
"touches gCtx / callBackend), got error: %v", err)
|
||||
}
|
||||
if id == 0 {
|
||||
t.Error("expected a non-zero generated job ID")
|
||||
}
|
||||
}
|
||||
|
|
@ -1,110 +1,144 @@
|
|||
package backend
|
||||
|
||||
import "testing"
|
||||
|
||||
// Linux errno + XCE error values. Plain ints keep this test cgo-free;
|
||||
// callBackend's fn returns a plain int by design for exactly this reason.
|
||||
const (
|
||||
ecnreset = -104 // ECONNRESET
|
||||
einval = -22 // EINVAL
|
||||
xceNotReady = -10013 // E_XCE_NOT_READY (controller init/electing)
|
||||
xceBadQueue = -10008 // E_XCE_BAD_QUEUE (semantic — must NOT trip)
|
||||
import (
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func resetBreaker() { MarkHealthy() }
|
||||
// health_test.go exercises health.go's degraded/healthy state machine
|
||||
// (component-failover-adoption task 4.5, boundary: ApiserverBackendHealth,
|
||||
// Req 4.1-4.2). Deliberately no build tag, matching health.go itself: pure
|
||||
// Go, no CGO -- runs under both CGO_ENABLED=0 and =1, so this file is the
|
||||
// one this package's tests can run without any backend/nix CGO setup at
|
||||
// all (see the task's own validation command: `CGO_ENABLED=0 go test
|
||||
// ./backend/...`).
|
||||
|
||||
// ── state transitions ────────────────────────────────────────────────────────
|
||||
// resetHealthForTest restores degradedUntilNanos to its zero value (never
|
||||
// degraded) between tests, mirroring reconnect_test.go's
|
||||
// resetNeedsRejoinForTest for the sibling needsRejoin state.
|
||||
func resetHealthForTest() {
|
||||
atomic.StoreInt64(°radedUntilNanos, 0)
|
||||
}
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// Req 4.1 -- basic degraded/healthy transitions
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
func TestIsDegraded_FreshState_ReportsHealthy(t *testing.T) {
|
||||
resetHealthForTest()
|
||||
|
||||
func TestCircuitBreakerStateTransitions(t *testing.T) {
|
||||
resetBreaker()
|
||||
if IsDegraded() {
|
||||
t.Fatal("fresh breaker should be healthy")
|
||||
t.Error("expected IsDegraded() to report false before MarkDegraded is ever called")
|
||||
}
|
||||
MarkDegraded()
|
||||
}
|
||||
|
||||
func TestMarkDegraded_ThenIsDegraded_ReportsTrue(t *testing.T) {
|
||||
resetHealthForTest()
|
||||
|
||||
MarkDegraded(1 * time.Minute)
|
||||
|
||||
if !IsDegraded() {
|
||||
t.Fatal("MarkDegraded should trip the breaker")
|
||||
t.Error("expected IsDegraded() to report true immediately after MarkDegraded")
|
||||
}
|
||||
}
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// design.md's own stated validation requirement: "unit tests cover
|
||||
// degraded->healthy transitions via both the TTL expiry and explicit
|
||||
// MarkHealthy()"
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
func TestMarkHealthy_ClearsDegradedStateExplicitly(t *testing.T) {
|
||||
resetHealthForTest()
|
||||
MarkDegraded(1 * time.Minute)
|
||||
if !IsDegraded() {
|
||||
t.Fatal("setup: expected degraded immediately after MarkDegraded")
|
||||
}
|
||||
|
||||
MarkHealthy()
|
||||
|
||||
if IsDegraded() {
|
||||
t.Fatal("MarkHealthy should clear the breaker")
|
||||
t.Error("expected IsDegraded() to report false immediately after MarkHealthy()")
|
||||
}
|
||||
}
|
||||
|
||||
// ── callBackend ──────────────────────────────────────────────────────────────
|
||||
func TestMarkDegraded_TTLExpiry_ClearsAutomaticallyWithoutMarkHealthy(t *testing.T) {
|
||||
resetHealthForTest()
|
||||
|
||||
func TestCallBackend_Degraded_FailsFastWithoutCalling(t *testing.T) {
|
||||
resetBreaker()
|
||||
MarkDegraded()
|
||||
|
||||
called := false
|
||||
err := callBackend("test", func() int {
|
||||
called = true
|
||||
return 0
|
||||
})
|
||||
if err == nil {
|
||||
t.Fatal("degraded call should return an error")
|
||||
}
|
||||
if called {
|
||||
t.Fatal("degraded call must NOT invoke the backend fn")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCallBackend_TransportError_TripsBreaker(t *testing.T) {
|
||||
resetBreaker()
|
||||
|
||||
err := callBackend("test", func() int { return ecnreset })
|
||||
if err == nil {
|
||||
t.Fatal("transport error should return an error")
|
||||
}
|
||||
MarkDegraded(20 * time.Millisecond)
|
||||
if !IsDegraded() {
|
||||
t.Fatal("transport error should trip the circuit breaker")
|
||||
t.Fatal("setup: expected degraded immediately after MarkDegraded")
|
||||
}
|
||||
|
||||
time.Sleep(60 * time.Millisecond)
|
||||
|
||||
if IsDegraded() {
|
||||
t.Error("expected IsDegraded() to report false once the TTL has elapsed, " +
|
||||
"without any explicit MarkHealthy() call -- the half-open circuit " +
|
||||
"breaker's own timeout must clear it")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCallBackend_XceNotReady_TripsBreaker(t *testing.T) {
|
||||
resetBreaker()
|
||||
func TestMarkDegraded_ExtendsAnExistingDegradedWindow(t *testing.T) {
|
||||
resetHealthForTest()
|
||||
|
||||
MarkDegraded(20 * time.Millisecond)
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
// A second call (e.g. a second call's own retry-budget exhaustion)
|
||||
// resets the window from now, not additively -- confirms MarkDegraded
|
||||
// always sets an absolute deadline, not a relative extension bug.
|
||||
MarkDegraded(20 * time.Millisecond)
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
|
||||
err := callBackend("test", func() int { return xceNotReady })
|
||||
if err == nil {
|
||||
t.Fatal("XCE not-ready should return an error")
|
||||
}
|
||||
if !IsDegraded() {
|
||||
t.Fatal("XCE not-ready (controller unavailable) should trip the circuit breaker")
|
||||
t.Error("expected still degraded 10ms into a freshly-reset 20ms window")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCallBackend_SemanticError_DoesNotTripBreaker(t *testing.T) {
|
||||
resetBreaker()
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// Concurrent-request safety (a go test -race concern, though -race itself
|
||||
// is known not to work in this ASAN-linked CGO environment -- see
|
||||
// tasks.md's task 4.2 Implementation Note. This proves race-freedom by
|
||||
// construction: every access goes through sync/atomic, so this test's
|
||||
// value is in exercising heavy concurrent read/write without a panic or
|
||||
// data corruption, not in relying on -race actually running clean here).
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
err := callBackend("test", func() int { return einval })
|
||||
if err == nil {
|
||||
t.Fatal("semantic error should return an error")
|
||||
}
|
||||
if IsDegraded() {
|
||||
t.Fatal("semantic error must NOT trip the circuit breaker")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCallBackend_XceSemanticError_DoesNotTripBreaker(t *testing.T) {
|
||||
resetBreaker()
|
||||
|
||||
err := callBackend("test", func() int { return xceBadQueue })
|
||||
if err == nil {
|
||||
t.Fatal("XCE semantic error should return an error")
|
||||
}
|
||||
if IsDegraded() {
|
||||
t.Fatal("XCE semantic rejection must NOT trip the circuit breaker")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCallBackend_Success_StaysHealthy(t *testing.T) {
|
||||
resetBreaker()
|
||||
|
||||
err := callBackend("test", func() int { return 0 })
|
||||
if err != nil {
|
||||
t.Fatalf("success should return nil, got %v", err)
|
||||
}
|
||||
if IsDegraded() {
|
||||
t.Fatal("success must not trip the circuit breaker")
|
||||
func TestHealthState_ConcurrentReadersAndWriters_NoDataCorruption(t *testing.T) {
|
||||
resetHealthForTest()
|
||||
|
||||
const goroutines = 50
|
||||
const iterations = 200
|
||||
|
||||
var wg sync.WaitGroup
|
||||
wg.Add(goroutines * 3)
|
||||
|
||||
for i := 0; i < goroutines; i++ {
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
for j := 0; j < iterations; j++ {
|
||||
_ = IsDegraded()
|
||||
}
|
||||
}()
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
for j := 0; j < iterations; j++ {
|
||||
MarkDegraded(time.Millisecond)
|
||||
}
|
||||
}()
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
for j := 0; j < iterations; j++ {
|
||||
MarkHealthy()
|
||||
}
|
||||
}()
|
||||
}
|
||||
|
||||
wg.Wait()
|
||||
// No assertion on the final state (inherently racy which goroutine's
|
||||
// write lands last) -- reaching here without a panic/deadlock, under
|
||||
// concurrent atomic reads and writes from 150 goroutines, is the proof
|
||||
// this test exists for.
|
||||
}
|
||||
|
|
|
|||
|
|
@ -18,35 +18,76 @@ func SetHeartbeatInterval(secs int) {
|
|||
}
|
||||
}
|
||||
|
||||
// needsRejoinAfterHeartbeat reports whether one heartbeat result should
|
||||
// trigger a backend rejoin + breaker clear. A failed heartbeat marks the
|
||||
// backend degraded and returns false; a successful heartbeat returns true
|
||||
// when EITHER a prior heartbeat failed (a transport reconnect just completed)
|
||||
// OR the circuit breaker is currently tripped. The breaker can be tripped by a
|
||||
// backend write failure (e.g. E_XCE_STANDBY while the controller is electing)
|
||||
// without any heartbeat ever failing — tracking only the heartbeat transition
|
||||
// would leave the breaker stuck degraded forever (bug-1513 follow-up).
|
||||
func needsRejoinAfterHeartbeat(hbOK bool, prevFailed *atomic.Bool) bool {
|
||||
if !hbOK {
|
||||
prevFailed.Store(true)
|
||||
MarkDegraded()
|
||||
return false
|
||||
}
|
||||
return prevFailed.Swap(false) || IsDegraded()
|
||||
}
|
||||
|
||||
// runHeartbeat loops until ctx is cancelled, calling fn(intervalSecs) on each tick.
|
||||
// Extracted from clib.go for testability without CGO.
|
||||
func runHeartbeat(ctx context.Context, fn func(int)) {
|
||||
// runHeartbeat loops until ctx is cancelled, calling fn(intervalSecs) on each
|
||||
// tick and classifying its returned rc as success (rc >= 0) or failure
|
||||
// (rc < 0) -- xcecli_heartbeat's own convention: it is XCECLI_FF_CALL-wrapped
|
||||
// (client_internal.h) and yields xce_heartbeat's rc verbatim, which is
|
||||
// either 0 (not yet due) or xce_nop's underlying send rc (xceapi.c) --
|
||||
// consistent with this codebase's established "rc < 0 is the only failure"
|
||||
// join/rejoin convention (tasks.md Implementation Notes, task 2.3).
|
||||
//
|
||||
// component-failover-adoption Phase 6 (bug-1548): replaces task 4.4's
|
||||
// original failure-to-success TRANSITION detector, which could never
|
||||
// observe the idle-period reconnect gap it was built to close.
|
||||
// XCECLI_FF_CALL -- the macro backing xcecli_heartbeat -- can rotate +
|
||||
// reconnect the transport ENTIRELY INSIDE a single call (while holding
|
||||
// ac->failover.lock), before evaluating its own EXPR, and still return
|
||||
// success. So a two-tick fail-then-succeed edge on fn's own return value is
|
||||
// not a reliable "a reconnect happened" signal: whenever a controller
|
||||
// election completes faster than the heartbeat interval -- the common
|
||||
// case, elections finish in single-digit seconds against a default 30s
|
||||
// interval -- the whole reconnect+recovery happens silently within one
|
||||
// tick and no failing tick is ever observed by this loop.
|
||||
//
|
||||
// currentReconnectGen reads endpoint_t.reconnect_gen (src/include/netio.h),
|
||||
// a monotonic counter bumped by xce_ctx_reconnect() (src/lib/xcectx.c) on
|
||||
// every successful transport reconnect, regardless of trigger path (write-
|
||||
// path XCECLI_FAILOVER_CALL retry or this heartbeat's own XCECLI_FF_CALL
|
||||
// best-effort self-heal) or whether the triggering call's own return code
|
||||
// ever surfaced a failure. Checked on EVERY tick, independent of that
|
||||
// tick's own fn() outcome -- a reconnect can complete on a tick whose own
|
||||
// send then fails for an unrelated reason, and that reconnect still needs
|
||||
// to be observed. Any change since the last-observed value means a
|
||||
// reconnect happened since this loop last checked, and fires
|
||||
// onReconnectDetected exactly once per change -- wired to reconnect.go's
|
||||
// setNeedsRejoin in production.
|
||||
//
|
||||
// onSuccessTick fires on every tick whose own fn() call succeeds -- wired
|
||||
// to checkAndPerformRejoin (task 4.3), which is itself a cheap no-op once
|
||||
// no rejoin is pending: this is what makes a rejoin attempt that itself
|
||||
// fails get retried on the next successful tick rather than abandoned
|
||||
// (Req 2 AC5). Gated on tick success, unlike the generation check above:
|
||||
// attempting a rejoin against a connection that just failed to even
|
||||
// heartbeat is not expected to succeed.
|
||||
//
|
||||
// currentReconnectGen and both callbacks are injected (rather than
|
||||
// referenced by name) so this loop's own tick-tracking logic stays
|
||||
// testable without CGO -- see heartbeat_test.go for that coverage and
|
||||
// heartbeat_rejoin_test.go (//go:build cgo) for the end-to-end wiring
|
||||
// against the real setNeedsRejoin/checkAndPerformRejoin. clib.go's
|
||||
// StartHeartbeat wires the real production callbacks and a real
|
||||
// currentReconnectGen reading C.xcecli_get_my_endpoint(gCtx.ctx).
|
||||
func runHeartbeat(ctx context.Context, fn func(int) int, currentReconnectGen func() uint64, onReconnectDetected, onSuccessTick func()) {
|
||||
interval := int(atomic.LoadInt32(&heartbeatIntervalSecs))
|
||||
t := time.NewTimer(time.Duration(interval) * time.Second)
|
||||
defer t.Stop()
|
||||
|
||||
lastGen := currentReconnectGen()
|
||||
for {
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return
|
||||
case <-t.C:
|
||||
fn(interval)
|
||||
succeeded := fn(interval) >= 0
|
||||
|
||||
if gen := currentReconnectGen(); gen != lastGen {
|
||||
lastGen = gen
|
||||
onReconnectDetected()
|
||||
}
|
||||
|
||||
if succeeded {
|
||||
onSuccessTick()
|
||||
}
|
||||
}
|
||||
interval = int(atomic.LoadInt32(&heartbeatIntervalSecs))
|
||||
t.Reset(time.Duration(interval) * time.Second)
|
||||
|
|
|
|||
196
src/frontend/apiserver/backend/heartbeat_rejoin_test.go
Normal file
196
src/frontend/apiserver/backend/heartbeat_rejoin_test.go
Normal file
|
|
@ -0,0 +1,196 @@
|
|||
//go:build cgo
|
||||
|
||||
package backend
|
||||
|
||||
import (
|
||||
"context"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// heartbeat_rejoin_test.go exercises runHeartbeat wired to reconnect.go's
|
||||
// REAL setNeedsRejoin/checkAndPerformRejoin (component-failover-adoption
|
||||
// task 4.4/Phase 6, boundary: ApiserverBackendBridge, Req 2 AC4/AC5) --
|
||||
// proving the idle-period rejoin trigger end-to-end at real tick
|
||||
// granularity, the same way clib.go's StartHeartbeat wires them in
|
||||
// production (currentReconnectGen itself -- the real
|
||||
// C.xcecli_get_my_endpoint(gCtx.ctx) read -- is NOT exercised here; these
|
||||
// tests inject a scripted currentReconnectGen double, same as
|
||||
// heartbeat_test.go, since gCtx has no live connection in a unit test).
|
||||
// heartbeat_test.go covers runHeartbeat's own tick/generation-tracking
|
||||
// logic via generic injected callbacks, without CGO; this file proves
|
||||
// those callbacks are wired to the right production functions and that the
|
||||
// composition actually clears/retains needsRejoin correctly across real
|
||||
// ticks.
|
||||
//
|
||||
// Kept //go:build cgo since needsRejoin/setNeedsRejoin/checkAndPerformRejoin/
|
||||
// performRejoinFn only exist in the CGO build (reconnect.go, task 4.3) --
|
||||
// this file itself does not `import "C"` (a _test.go file can never do that
|
||||
// directly, golang/go#13938), matching the reconnect_test.go/candidates_test.go
|
||||
// precedent already established in this package.
|
||||
//
|
||||
// performRejoinFn (reconnect.go's existing test seam, task 4.3) is
|
||||
// overridden here to script the actual xcecli_join outcome without a live
|
||||
// controller -- same technique reconnect_test.go's own retry-on-failure
|
||||
// tests use. scriptedHeartbeatFn/scriptedReconnectGen (heartbeat_test.go)
|
||||
// script the heartbeat send outcomes and the reconnect-generation reads.
|
||||
|
||||
// This is the CGO-level counterpart to
|
||||
// TestRunHeartbeat_ReconnectDetected_FiresOnGenChangeEvenWhenAllTicksSucceed
|
||||
// in heartbeat_test.go, and is the actual proof of closure for bug-1548 at
|
||||
// the level production code runs at: every heartbeat tick's own fn() call
|
||||
// reports SUCCESS (the literal shape XCECLI_FF_CALL's silent inline
|
||||
// reconnect+recover produces -- no failing tick is ever observed), yet a
|
||||
// reconnect genuinely happened (the generation counter changed). The old
|
||||
// failure-to-success transition detector could never set needsRejoin in
|
||||
// this scenario; this test proves the replacement generation-based
|
||||
// detector does. Still only proof of the DETECTOR's logic wired to the
|
||||
// real production callbacks -- whether currentReconnectGen's real
|
||||
// C.xcecli_get_my_endpoint read is itself reachable/reliable against a real
|
||||
// XCECLI_FF_CALL reconnect is proven only by the required live E2E re-run
|
||||
// of task 5.2's scenario (Phase 6, task 6.2), not by any unit test.
|
||||
func TestRunHeartbeat_IdlePeriodRecovery_GenChangeAloneSetsNeedsRejoinEvenWhenAllTicksSucceed(t *testing.T) {
|
||||
captureLog(t) // checkAndPerformRejoin logs via utils.Log; nil otherwise
|
||||
resetNeedsRejoinForTest()
|
||||
atomic.StoreInt32(&heartbeatIntervalSecs, 1)
|
||||
|
||||
origPerformRejoinFn := performRejoinFn
|
||||
defer func() { performRejoinFn = origPerformRejoinFn }()
|
||||
|
||||
var joinCalls int32
|
||||
joinDone := make(chan struct{}, 8)
|
||||
performRejoinFn = func() int {
|
||||
atomic.AddInt32(&joinCalls, 1)
|
||||
joinDone <- struct{}{}
|
||||
return 0 // succeeds on the first attempt
|
||||
}
|
||||
|
||||
// Every heartbeat tick this test observes reports success -- no failure
|
||||
// ever surfaces to Go, matching XCECLI_FF_CALL's silent self-heal.
|
||||
fn := scriptedHeartbeatFn([]int{0, 0, 0})
|
||||
// Generation changes on the second tick (pre-loop baseline=5, tick1=5
|
||||
// unchanged, tick2=6 changed) -- a reconnect happened even though the
|
||||
// caller never saw a failing tick.
|
||||
gens := scriptedReconnectGen([]uint64{5, 5, 6, 6})
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
runHeartbeat(ctx, fn, gens, setNeedsRejoin, checkAndPerformRejoin)
|
||||
close(done)
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-joinDone:
|
||||
case <-time.After(6 * time.Second):
|
||||
t.Fatal("timed out waiting for the rejoin attempt triggered by the generation change")
|
||||
}
|
||||
cancel()
|
||||
<-done
|
||||
|
||||
if got := atomic.LoadInt32(&joinCalls); got != 1 {
|
||||
t.Errorf("performRejoinFn called %d times, want exactly 1", got)
|
||||
}
|
||||
if got := atomic.LoadInt32(&needsRejoin); got != 0 {
|
||||
t.Error("expected needsRejoin to clear once the rejoin succeeds")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunHeartbeat_RejoinRetriesOnFailureUntilItSucceeds(t *testing.T) {
|
||||
captureLog(t)
|
||||
resetNeedsRejoinForTest()
|
||||
atomic.StoreInt32(&heartbeatIntervalSecs, 1)
|
||||
|
||||
origPerformRejoinFn := performRejoinFn
|
||||
defer func() { performRejoinFn = origPerformRejoinFn }()
|
||||
|
||||
var joinCalls int32
|
||||
joinDone := make(chan struct{}, 8)
|
||||
performRejoinFn = func() int {
|
||||
n := atomic.AddInt32(&joinCalls, 1)
|
||||
joinDone <- struct{}{}
|
||||
if n == 1 {
|
||||
return -1 // simulates -E_XCE_NOT_READY: this attempt's own retry budget exhausted
|
||||
}
|
||||
return 0 // succeeds on the second, separate attempt
|
||||
}
|
||||
|
||||
fn := scriptedHeartbeatFn([]int{0, 0, 0})
|
||||
// Generation changes once, on tick 1 (baseline=1, tick1=2 changed,
|
||||
// stays 2 thereafter) -- one reconnect, but the resulting rejoin must
|
||||
// be retried across multiple successful ticks until it succeeds (Req 2
|
||||
// AC5), not abandoned after its first failed attempt.
|
||||
gens := scriptedReconnectGen([]uint64{1, 2, 2, 2})
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
runHeartbeat(ctx, fn, gens, setNeedsRejoin, checkAndPerformRejoin)
|
||||
close(done)
|
||||
}()
|
||||
|
||||
for i := 0; i < 2; i++ {
|
||||
select {
|
||||
case <-joinDone:
|
||||
case <-time.After(6 * time.Second):
|
||||
t.Fatalf("timed out waiting for rejoin attempt %d", i+1)
|
||||
}
|
||||
}
|
||||
cancel()
|
||||
<-done
|
||||
|
||||
if got := atomic.LoadInt32(&joinCalls); got != 2 {
|
||||
t.Errorf("performRejoinFn called %d times, want exactly 2 (first attempt "+
|
||||
"fails, retried on the next successful heartbeat tick)", got)
|
||||
}
|
||||
if got := atomic.LoadInt32(&needsRejoin); got != 0 {
|
||||
t.Error("expected needsRejoin to clear once the retried rejoin succeeds")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunHeartbeat_NoGenChange_NeverSetsNeedsRejoin(t *testing.T) {
|
||||
resetNeedsRejoinForTest()
|
||||
atomic.StoreInt32(&heartbeatIntervalSecs, 1)
|
||||
|
||||
origPerformRejoinFn := performRejoinFn
|
||||
defer func() { performRejoinFn = origPerformRejoinFn }()
|
||||
|
||||
var joinCalls int32
|
||||
performRejoinFn = func() int {
|
||||
atomic.AddInt32(&joinCalls, 1)
|
||||
return 0
|
||||
}
|
||||
|
||||
tickEvents := make(chan struct{}, 8)
|
||||
fn := func(_ int) int {
|
||||
tickEvents <- struct{}{}
|
||||
return 0 // always succeeds
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
runHeartbeat(ctx, fn, constantReconnectGen(9), setNeedsRejoin, checkAndPerformRejoin)
|
||||
close(done)
|
||||
}()
|
||||
|
||||
for i := 0; i < 3; i++ {
|
||||
select {
|
||||
case <-tickEvents:
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatalf("timed out waiting for tick %d", i+1)
|
||||
}
|
||||
}
|
||||
cancel()
|
||||
<-done
|
||||
|
||||
if got := atomic.LoadInt32(&needsRejoin); got != 0 {
|
||||
t.Error("needsRejoin must stay clear when the reconnect generation never changes -- " +
|
||||
"no reconnect ever occurred")
|
||||
}
|
||||
if got := atomic.LoadInt32(&joinCalls); got != 0 {
|
||||
t.Errorf("performRejoinFn called %d times, want 0 (checkAndPerformRejoin "+
|
||||
"must no-op when no rejoin is pending)", got)
|
||||
}
|
||||
}
|
||||
|
|
@ -31,97 +31,36 @@ func TestSetHeartbeatInterval_NegativeIgnored(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestNeedsRejoinAfterHeartbeat_FailureMarksDegraded(t *testing.T) {
|
||||
var prev atomic.Bool
|
||||
resetBreaker()
|
||||
|
||||
got := needsRejoinAfterHeartbeat(false, &prev)
|
||||
if got {
|
||||
t.Error("failed heartbeat should not request rejoin")
|
||||
}
|
||||
if !IsDegraded() {
|
||||
t.Error("failed heartbeat should trip the circuit breaker")
|
||||
}
|
||||
if !prev.Load() {
|
||||
t.Error("failed heartbeat should record the failure for the next transition")
|
||||
}
|
||||
// constantReconnectGen returns a currentReconnectGen double that never
|
||||
// changes value -- used by tests that only care about runHeartbeat's
|
||||
// tick/cancellation mechanics or fn's own success/failure classification,
|
||||
// not its reconnect-detection behavior (see the Phase 6 tests below for
|
||||
// that coverage).
|
||||
func constantReconnectGen(gen uint64) func() uint64 {
|
||||
return func() uint64 { return gen }
|
||||
}
|
||||
|
||||
func TestNeedsRejoinAfterHeartbeat_HealthySuccessNoOp(t *testing.T) {
|
||||
var prev atomic.Bool
|
||||
resetBreaker()
|
||||
|
||||
got := needsRejoinAfterHeartbeat(true, &prev)
|
||||
if got {
|
||||
t.Error("successful heartbeat with healthy breaker and no prior failure should not rejoin")
|
||||
}
|
||||
}
|
||||
|
||||
func TestNeedsRejoinAfterHeartbeat_TransportRecoveryRejoins(t *testing.T) {
|
||||
var prev atomic.Bool
|
||||
resetBreaker()
|
||||
|
||||
needsRejoinAfterHeartbeat(false, &prev) // transport failed
|
||||
if !IsDegraded() {
|
||||
t.Fatal("precondition: breaker should be tripped")
|
||||
}
|
||||
|
||||
got := needsRejoinAfterHeartbeat(true, &prev) // transport recovered
|
||||
if !got {
|
||||
t.Error("successful heartbeat after a failure should request rejoin")
|
||||
}
|
||||
// The caller (StartHeartbeat) clears the breaker when the helper says rejoin.
|
||||
MarkHealthy()
|
||||
if IsDegraded() {
|
||||
t.Error("caller must clear the breaker after the recovery transition")
|
||||
}
|
||||
}
|
||||
|
||||
// bug-1513 follow-up: the breaker can be tripped by a backend write failure
|
||||
// (e.g. E_XCE_STANDBY while the controller is electing) with no heartbeat ever
|
||||
// failing. A successful heartbeat must still clear it, or the backend stays
|
||||
// degraded forever until a manual apiserver restart.
|
||||
func TestNeedsRejoinAfterHeartbeat_BreakerTrippedNoHeartbeatFailure(t *testing.T) {
|
||||
var prev atomic.Bool
|
||||
resetBreaker()
|
||||
|
||||
MarkDegraded() // tripped by a backend write failure, not by a heartbeat
|
||||
|
||||
got := needsRejoinAfterHeartbeat(true, &prev)
|
||||
if !got {
|
||||
t.Error("successful heartbeat must clear a breaker tripped by a backend write failure")
|
||||
}
|
||||
}
|
||||
|
||||
func TestNeedsRejoinAfterHeartbeat_RecoveryResetsPrevFailed(t *testing.T) {
|
||||
var prev atomic.Bool
|
||||
resetBreaker()
|
||||
|
||||
if needsRejoinAfterHeartbeat(true, &prev) {
|
||||
MarkHealthy() // simulate the caller clearing the breaker
|
||||
}
|
||||
|
||||
if prev.Load() {
|
||||
t.Error("prevFailed should be reset after the success transition")
|
||||
}
|
||||
|
||||
// Next success with a healthy breaker must be a no-op (no repeated rejoin).
|
||||
got := needsRejoinAfterHeartbeat(true, &prev)
|
||||
if got {
|
||||
t.Error("second consecutive successful heartbeat should not rejoin again")
|
||||
}
|
||||
// noopRejoinHooks are used by tests that only care about runHeartbeat's
|
||||
// tick/cancellation mechanics, not its reconnect-detected/pending-rejoin
|
||||
// callback behavior (see the Phase 6 tests below for that coverage).
|
||||
func noopRejoinHooks() (func(), func()) {
|
||||
return func() {}, func() {}
|
||||
}
|
||||
|
||||
func TestRunHeartbeat_CallsFunction(t *testing.T) {
|
||||
atomic.StoreInt32(&heartbeatIntervalSecs, 1)
|
||||
|
||||
var callCount int64
|
||||
fn := func(_ int) { atomic.AddInt64(&callCount, 1) }
|
||||
fn := func(_ int) int {
|
||||
atomic.AddInt64(&callCount, 1)
|
||||
return 0
|
||||
}
|
||||
onReconnectDetected, onSuccessTick := noopRejoinHooks()
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
runHeartbeat(ctx, fn)
|
||||
runHeartbeat(ctx, fn, constantReconnectGen(0), onReconnectDetected, onSuccessTick)
|
||||
close(done)
|
||||
}()
|
||||
|
||||
|
|
@ -139,13 +78,15 @@ func TestRunHeartbeat_CancelsOnContext(t *testing.T) {
|
|||
cancel() // already cancelled
|
||||
|
||||
var callCount int64
|
||||
fn := func(_ int) {
|
||||
fn := func(_ int) int {
|
||||
atomic.AddInt64(&callCount, 1)
|
||||
return 0
|
||||
}
|
||||
onReconnectDetected, onSuccessTick := noopRejoinHooks()
|
||||
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
runHeartbeat(ctx, fn)
|
||||
runHeartbeat(ctx, fn, constantReconnectGen(0), onReconnectDetected, onSuccessTick)
|
||||
close(done)
|
||||
}()
|
||||
|
||||
|
|
@ -160,3 +101,185 @@ func TestRunHeartbeat_CancelsOnContext(t *testing.T) {
|
|||
t.Errorf("fn was called %d times, want 0 for already-cancelled context", got)
|
||||
}
|
||||
}
|
||||
|
||||
// scriptedHeartbeatFn returns a runHeartbeat fn double that yields
|
||||
// results[0], results[1], ... on successive calls, clamping to the last
|
||||
// entry for any calls beyond len(results) (defensive against a stray extra
|
||||
// tick firing before a test's cancel() takes effect -- see the tests below,
|
||||
// which all stop reading/cancel as soon as the expected callback events are
|
||||
// observed rather than relying on a fixed sleep window).
|
||||
func scriptedHeartbeatFn(results []int) func(int) int {
|
||||
var idx int32
|
||||
return func(_ int) int {
|
||||
i := atomic.AddInt32(&idx, 1) - 1
|
||||
if int(i) >= len(results) {
|
||||
i = int32(len(results) - 1)
|
||||
}
|
||||
return results[i]
|
||||
}
|
||||
}
|
||||
|
||||
// scriptedReconnectGen returns a currentReconnectGen double that yields
|
||||
// gens[0], gens[1], ... on successive calls (one call per tick -- runHeartbeat
|
||||
// reads it exactly once per tick, after fn), clamping to the last entry for
|
||||
// any calls beyond len(gens). The very first call (the pre-loop baseline
|
||||
// read in runHeartbeat, before any tick) is NOT scripted from this slice --
|
||||
// callers should include their intended pre-loop baseline as gens[0] and
|
||||
// note the first in-loop read is gens[1] onward matching a real
|
||||
// endpoint_t.reconnect_gen starting at a fixed value and changing on later
|
||||
// reconnects.
|
||||
func scriptedReconnectGen(gens []uint64) func() uint64 {
|
||||
var idx int32
|
||||
return func() uint64 {
|
||||
i := atomic.AddInt32(&idx, 1) - 1
|
||||
if int(i) >= len(gens) {
|
||||
i = int32(len(gens) - 1)
|
||||
}
|
||||
return gens[i]
|
||||
}
|
||||
}
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// component-failover-adoption Phase 6 (bug-1548) -- reconnect-generation-
|
||||
// based idle-period rejoin trigger, replacing task 4.4's original failure-
|
||||
// to-success transition detector (which structurally could not observe a
|
||||
// reconnect that completed silently inside a single XCECLI_FF_CALL). These
|
||||
// tests exercise runHeartbeat's own tick/generation-tracking logic via
|
||||
// generic injected callbacks, entirely without CGO (the production wiring
|
||||
// -- reconnect.go's real setNeedsRejoin/checkAndPerformRejoin plus clib.go's
|
||||
// real currentReconnectGen reading C.xcecli_get_my_endpoint -- is exercised
|
||||
// end-to-end in heartbeat_rejoin_test.go, //go:build cgo).
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
// This is the literal shape of bug-1548: every heartbeat tick the Go loop
|
||||
// observes reports SUCCESS (fn never returns a failure -- XCECLI_FF_CALL's
|
||||
// silent inline reconnect+recover means the caller's own return code never
|
||||
// surfaces one), yet a reconnect genuinely happened (the generation counter
|
||||
// changed). The old failure-to-success transition detector could never fire
|
||||
// here since it requires an observed failing tick first. This test is the
|
||||
// actual proof the fix closes the gap the old detector's own green test
|
||||
// suite could not catch -- it is still, deliberately, only proof of the
|
||||
// DETECTOR's logic: whether the generation counter is reachable/reliable
|
||||
// against the real XCECLI_FF_CALL path is proven only by a live E2E re-run
|
||||
// (component-failover-adoption Phase 6, task 6.2), not by this test.
|
||||
func TestRunHeartbeat_ReconnectDetected_FiresOnGenChangeEvenWhenAllTicksSucceed(t *testing.T) {
|
||||
atomic.StoreInt32(&heartbeatIntervalSecs, 1)
|
||||
|
||||
fn := scriptedHeartbeatFn([]int{0, 0, 0}) // every tick succeeds
|
||||
gens := scriptedReconnectGen([]uint64{5, 5, 6, 6})
|
||||
|
||||
events := make(chan string, 8)
|
||||
onReconnectDetected := func() { events <- "reconnect_detected" }
|
||||
onSuccessTick := func() { events <- "tick" }
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
runHeartbeat(ctx, fn, gens, onReconnectDetected, onSuccessTick)
|
||||
close(done)
|
||||
}()
|
||||
|
||||
// tick 1: gen 5->5 (no change), tick succeeds -> "tick"
|
||||
// tick 2: gen 5->6 (changed) -> "reconnect_detected", then tick succeeds -> "tick"
|
||||
want := []string{"tick", "reconnect_detected", "tick"}
|
||||
got := make([]string, 0, len(want))
|
||||
for range want {
|
||||
select {
|
||||
case e := <-events:
|
||||
got = append(got, e)
|
||||
case <-time.After(6 * time.Second):
|
||||
t.Fatalf("timed out waiting for heartbeat callback events, got %v so far", got)
|
||||
}
|
||||
}
|
||||
cancel()
|
||||
<-done
|
||||
|
||||
for i := range want {
|
||||
if got[i] != want[i] {
|
||||
t.Fatalf("callback event %d = %q, want %q (full sequence got=%v want=%v) -- "+
|
||||
"onReconnectDetected must fire on a reconnect_gen change even though "+
|
||||
"every tick's own fn() call reported success",
|
||||
i, got[i], want[i], got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A reconnect can complete on a tick whose own send then fails for an
|
||||
// unrelated reason -- the generation check must not be gated on tick
|
||||
// success, unlike onSuccessTick.
|
||||
func TestRunHeartbeat_GenChangeOnFailedTick_StillFiresOnReconnectDetected(t *testing.T) {
|
||||
atomic.StoreInt32(&heartbeatIntervalSecs, 1)
|
||||
|
||||
fn := scriptedHeartbeatFn([]int{-1}) // the one tick this test observes fails
|
||||
gens := scriptedReconnectGen([]uint64{1, 2})
|
||||
|
||||
events := make(chan string, 8)
|
||||
onReconnectDetected := func() { events <- "reconnect_detected" }
|
||||
onSuccessTick := func() { events <- "tick" }
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
runHeartbeat(ctx, fn, gens, onReconnectDetected, onSuccessTick)
|
||||
close(done)
|
||||
}()
|
||||
|
||||
select {
|
||||
case e := <-events:
|
||||
if e != "reconnect_detected" {
|
||||
t.Fatalf("first event = %q, want %q -- a gen change must fire "+
|
||||
"onReconnectDetected even on a tick whose own fn() call failed", e, "reconnect_detected")
|
||||
}
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("timed out waiting for onReconnectDetected")
|
||||
}
|
||||
cancel()
|
||||
<-done
|
||||
|
||||
select {
|
||||
case e := <-events:
|
||||
t.Fatalf("unexpected extra event %q -- onSuccessTick must not fire on a failed tick", e)
|
||||
default:
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunHeartbeat_NoGenChange_NeverFiresOnReconnectDetected(t *testing.T) {
|
||||
atomic.StoreInt32(&heartbeatIntervalSecs, 1)
|
||||
|
||||
fn := scriptedHeartbeatFn([]int{0, 0, 0})
|
||||
|
||||
var reconnectCalls int32
|
||||
onReconnectDetected := func() { atomic.AddInt32(&reconnectCalls, 1) }
|
||||
|
||||
var tickCalls int32
|
||||
tickEvents := make(chan struct{}, 8)
|
||||
onSuccessTick := func() {
|
||||
atomic.AddInt32(&tickCalls, 1)
|
||||
tickEvents <- struct{}{}
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
runHeartbeat(ctx, fn, constantReconnectGen(42), onReconnectDetected, onSuccessTick)
|
||||
close(done)
|
||||
}()
|
||||
|
||||
for i := 0; i < 3; i++ {
|
||||
select {
|
||||
case <-tickEvents:
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatalf("timed out waiting for tick %d", i+1)
|
||||
}
|
||||
}
|
||||
cancel()
|
||||
<-done
|
||||
|
||||
if got := atomic.LoadInt32(&reconnectCalls); got != 0 {
|
||||
t.Errorf("onReconnectDetected called %d times, want 0 -- the generation "+
|
||||
"never changed, so no reconnect ever happened", got)
|
||||
}
|
||||
if got := atomic.LoadInt32(&tickCalls); got < 3 {
|
||||
t.Errorf("onSuccessTick called %d times, want at least 3", got)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ import (
|
|||
// resetIntegrationStores reinitializes all global stores for test isolation.
|
||||
func resetIntegrationStores() {
|
||||
store.InitStores()
|
||||
resetHostConfigCache()
|
||||
utils.Log = zap.NewNop()
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,24 +0,0 @@
|
|||
package backend
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net"
|
||||
"strconv"
|
||||
)
|
||||
|
||||
// parsePeer splits a "host:port" string. Uses net.SplitHostPort so IPv6
|
||||
// bracket notation (e.g. "[::1]:9990") is handled correctly.
|
||||
func parsePeer(peer string) (host string, port int, err error) {
|
||||
h, portStr, err := net.SplitHostPort(peer)
|
||||
if err != nil {
|
||||
return "", 0, fmt.Errorf("invalid peer %q: %w", peer, err)
|
||||
}
|
||||
if h == "" {
|
||||
return "", 0, fmt.Errorf("invalid peer %q: empty host", peer)
|
||||
}
|
||||
port, err = strconv.Atoi(portStr)
|
||||
if err != nil || port <= 0 || port > 65535 {
|
||||
return "", 0, fmt.Errorf("invalid port in peer %q", peer)
|
||||
}
|
||||
return h, port, nil
|
||||
}
|
||||
|
|
@ -1,95 +0,0 @@
|
|||
package backend
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestParsePeer(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
peer string
|
||||
wantHost string
|
||||
wantPort int
|
||||
wantError bool
|
||||
}{
|
||||
{
|
||||
name: "valid simple host:port",
|
||||
peer: "ctrl01:9990",
|
||||
wantHost: "ctrl01",
|
||||
wantPort: 9990,
|
||||
},
|
||||
{
|
||||
name: "valid IPv4",
|
||||
peer: "192.168.1.10:9990",
|
||||
wantHost: "192.168.1.10",
|
||||
wantPort: 9990,
|
||||
},
|
||||
{
|
||||
name: "valid FQDN",
|
||||
peer: "ctrl01.cluster.local:9990",
|
||||
wantHost: "ctrl01.cluster.local",
|
||||
wantPort: 9990,
|
||||
},
|
||||
{
|
||||
name: "min valid port",
|
||||
peer: "host:1",
|
||||
wantHost: "host",
|
||||
wantPort: 1,
|
||||
},
|
||||
{
|
||||
name: "max valid port",
|
||||
peer: "host:65535",
|
||||
wantHost: "host",
|
||||
wantPort: 65535,
|
||||
},
|
||||
{
|
||||
name: "missing colon",
|
||||
peer: "ctrl019990",
|
||||
wantError: true,
|
||||
},
|
||||
{
|
||||
name: "port zero",
|
||||
peer: "ctrl01:0",
|
||||
wantError: true,
|
||||
},
|
||||
{
|
||||
name: "port out of range",
|
||||
peer: "ctrl01:65536",
|
||||
wantError: true,
|
||||
},
|
||||
{
|
||||
name: "non-numeric port",
|
||||
peer: "ctrl01:abc",
|
||||
wantError: true,
|
||||
},
|
||||
{
|
||||
name: "colon at start (empty host)",
|
||||
peer: ":9990",
|
||||
wantError: true,
|
||||
},
|
||||
{
|
||||
name: "empty string",
|
||||
peer: "",
|
||||
wantError: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
host, port, err := parsePeer(tc.peer)
|
||||
if tc.wantError {
|
||||
if err == nil {
|
||||
t.Errorf("parsePeer(%q): expected error, got host=%q port=%d", tc.peer, host, port)
|
||||
}
|
||||
return
|
||||
}
|
||||
if err != nil {
|
||||
t.Fatalf("parsePeer(%q): unexpected error: %v", tc.peer, err)
|
||||
}
|
||||
if host != tc.wantHost {
|
||||
t.Errorf("host: got %q, want %q", host, tc.wantHost)
|
||||
}
|
||||
if port != tc.wantPort {
|
||||
t.Errorf("port: got %d, want %d", port, tc.wantPort)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
160
src/frontend/apiserver/backend/reconnect.go
Normal file
160
src/frontend/apiserver/backend/reconnect.go
Normal file
|
|
@ -0,0 +1,160 @@
|
|||
//go:build cgo
|
||||
|
||||
package backend
|
||||
|
||||
/*
|
||||
#include "client.h"
|
||||
#include "controller.capnp.h"
|
||||
*/
|
||||
import "C"
|
||||
|
||||
import (
|
||||
logger "apiserver/utils"
|
||||
"sync/atomic"
|
||||
"unsafe"
|
||||
|
||||
"go.uber.org/zap"
|
||||
)
|
||||
|
||||
// reconnect.go implements apiserver's rejoin-after-transparent-reconnect
|
||||
// logic (component-failover-adoption task 4.3, boundary:
|
||||
// ApiserverBackendBridge, Req 2 AC1-3/AC5). See design.md's "Rejoin
|
||||
// callback constraint" section and clib.go's xce_backend_init() for the
|
||||
// C-side shim/extern/registration this file's exported flag-setter is
|
||||
// wired to. Per that shim's own doc comment: cgo symbol resolution is
|
||||
// per-file (see wire_dispatch_support.go's documented rationale for the
|
||||
// identical go_push_handler precedent), so the shim/extern/registration
|
||||
// must live in clib.go, not here -- this file holds only the plain Go
|
||||
// logic (Go-level //export functions are visible package-wide once
|
||||
// generated, unlike the raw C shim/extern that only resolves within the
|
||||
// C translation unit of the file that declares it).
|
||||
//
|
||||
// Concurrency note: unlike worker's/scheduler's single-threaded C daemons
|
||||
// (where a plain bool is safe), apiserver serves concurrent HTTP requests
|
||||
// -- SubmitJob/ControlJob/ControlQueue/ControlHost/FetchFile can all run on
|
||||
// different goroutines simultaneously. needsRejoin is therefore a proper
|
||||
// sync/atomic int32, the same pattern already used by heartbeatIntervalSecs
|
||||
// (heartbeat.go), not a plain bool.
|
||||
|
||||
// needsRejoin flags that the client library has transparently reconnected
|
||||
// the underlying transport and apiserver must re-issue xcecli_join() as a
|
||||
// separate, later call before it is a cluster member again. 0 = no rejoin
|
||||
// pending, 1 = rejoin pending. Set by setNeedsRejoin() (invoked from the
|
||||
// library while its own lock is held); cleared only once performRejoinFn()
|
||||
// itself succeeds (Req 2 AC5) -- never merely because it was attempted.
|
||||
var needsRejoin int32
|
||||
|
||||
// setNeedsRejoin is the flag-setting logic behind the exported
|
||||
// go_backend_rejoin_handler below, factored out into a plain-Go-typed
|
||||
// function so it is directly callable (and independently timeable) from
|
||||
// reconnect_test.go: a _test.go file can never `import "C"` itself
|
||||
// (golang/go#13938), so it cannot call go_backend_rejoin_handler directly
|
||||
// (its signature involves C.int) -- same clib.go/clib_test.go split this
|
||||
// package already established for currentClusterCandidates()/
|
||||
// currentClientConfigNMasters().
|
||||
func setNeedsRejoin() {
|
||||
atomic.StoreInt32(&needsRejoin, 1)
|
||||
}
|
||||
|
||||
// go_backend_rejoin_handler is the flag-setter registered as the client
|
||||
// library's xcecli_reconnect_cb, via the C shim (xce_backend_rejoin_handler)
|
||||
// and xcecli_set_reconnect_handler() call that live in clib.go's
|
||||
// xce_backend_init() (see that function's own doc comment for why
|
||||
// registration happens AFTER xcecli_start() succeeds, not before).
|
||||
//
|
||||
// The client library invokes this SYNCHRONOUSLY, on the calling goroutine,
|
||||
// while still holding its own per-ctx lock (ac->failover.lock, a
|
||||
// non-recursive mutex held for the whole XCECLI_FAILOVER_CALL retry loop --
|
||||
// confirmed directly against client_failover.c:341-411,
|
||||
// xcecli_invoke_rejoin() at line 397). Per design.md's "Rejoin callback
|
||||
// constraint", this function performs NO I/O and calls NO xcecli_*
|
||||
// function: doing so would try to re-acquire the same already-held lock
|
||||
// and self-deadlock the calling goroutine forever. It only sets a flag
|
||||
// (via setNeedsRejoin) and returns 0 immediately so the interrupted
|
||||
// operation resumes normally -- the actual rejoin happens later, as a
|
||||
// separate call, once the triggering call has returned and the lock is
|
||||
// free (see performPendingRejoin/checkAndPerformRejoin below).
|
||||
//
|
||||
//export go_backend_rejoin_handler
|
||||
func go_backend_rejoin_handler(ctx unsafe.Pointer, arg unsafe.Pointer) C.int {
|
||||
setNeedsRejoin()
|
||||
return 0
|
||||
}
|
||||
|
||||
// performPendingRejoin issues the actual xcecli_join() call: the same join
|
||||
// payload used at startup (xce_backend_init, clib.go), as a separate,
|
||||
// subsequent call once the triggering xcecli_* call has already returned
|
||||
// and the client library's internal failover lock is free.
|
||||
//
|
||||
// Returns the raw rc as a plain Go int (not C.int) so it can be assigned
|
||||
// to performRejoinFn -- a pure-Go-typed function variable tests override
|
||||
// with a controllable double, avoiding the need for a live controller or
|
||||
// a -Wl,--wrap=-style linker interposition to exercise retry-on-failure.
|
||||
//
|
||||
// Does not loop/retry on its own failure: retry-on-failure is achieved by
|
||||
// leaving needsRejoin set so the NEXT backend call's post-call check
|
||||
// (checkAndPerformRejoin) retries it -- the same "compose with the
|
||||
// existing per-call cadence, no separate timer" pattern worker's
|
||||
// checking_connection()/rejoin_call and scheduler's task 3.2 rejoin-check
|
||||
// composition both already use.
|
||||
//
|
||||
// Success is rc >= 0, NOT rc == 0 (matches worker's/scheduler's
|
||||
// established convention, tasks.md Implementation Notes for task 2.3): a
|
||||
// successful join returns the controller's reply.status verbatim, which
|
||||
// can be a non-negative assigned slot/identity value, not always exactly
|
||||
// 0. Only a genuine failure reply.status is negative.
|
||||
func performPendingRejoin() int {
|
||||
rc := C.xcecli_join(gCtx.ctx, C.int(C.HostRole_apiserver), nil)
|
||||
return int(rc)
|
||||
}
|
||||
|
||||
// performRejoinFn defaults to the real production call
|
||||
// (performPendingRejoin above). Tests override it with a controllable
|
||||
// double so checkAndPerformRejoin()'s retry-until-success composition can
|
||||
// be verified deterministically -- mirrors worker's identical rejoin_call
|
||||
// function-pointer testability pattern (controller_connector.c's
|
||||
// perform_pending_rejoin/rejoin_call).
|
||||
var performRejoinFn = performPendingRejoin
|
||||
|
||||
// checkAndPerformRejoin is the shared post-call helper every backend-call
|
||||
// wrapper (SubmitJob, ControlJob, ControlQueue, ControlHost, FetchFile --
|
||||
// see clib.go) calls once its own underlying C.xcecli_* call has returned
|
||||
// (Req 2 AC1-3, AC5): if a rejoin is pending, issue it as a separate step,
|
||||
// clearing the pending indicator only if that call succeeds. On its own
|
||||
// failure, the indicator stays set so the next backend call's return
|
||||
// retries it -- never abandoned.
|
||||
//
|
||||
// Runs regardless of the triggering call's own outcome (mirrors worker's
|
||||
// checking_connection(): a transparent reconnect can be pending
|
||||
// independent of whether this particular call happened to succeed).
|
||||
//
|
||||
// This is deliberately only the POST-call half of what design.md describes
|
||||
// as the eventual shared helper -- the PRE-call fail-fast check
|
||||
// (IsDegraded(), health.go) lives in clib.go's callBackend (task 4.5), not
|
||||
// here: checkAndPerformRejoin() itself never reads IsDegraded() and must
|
||||
// not (see health.go's own doc comment for why the two states must stay
|
||||
// independent -- gating this function's retry on "still degraded" would
|
||||
// let the circuit breaker's TTL silently stop retrying a join that never
|
||||
// actually succeeded).
|
||||
//
|
||||
// On success, also calls health.go's MarkHealthy() (task 4.5, Req 4.1/4.2):
|
||||
// this is the single call site both the write-path (task 4.3, via each
|
||||
// backend-call wrapper's post-call check) and the idle-period heartbeat
|
||||
// path (task 4.4, via runHeartbeat's onSuccessTick) funnel through, so one
|
||||
// MarkHealthy() call here covers a successful rejoin from either path.
|
||||
func checkAndPerformRejoin() {
|
||||
if atomic.LoadInt32(&needsRejoin) == 0 {
|
||||
return
|
||||
}
|
||||
|
||||
rc := performRejoinFn()
|
||||
if rc >= 0 {
|
||||
atomic.StoreInt32(&needsRejoin, 0)
|
||||
MarkHealthy()
|
||||
logger.Info("checkAndPerformRejoin: rejoin after transparent reconnect succeeded")
|
||||
return
|
||||
}
|
||||
|
||||
logger.Warn("checkAndPerformRejoin: rejoin after transparent reconnect failed; will retry on next backend call",
|
||||
zap.Int("rc", rc))
|
||||
}
|
||||
374
src/frontend/apiserver/backend/reconnect_test.go
Normal file
374
src/frontend/apiserver/backend/reconnect_test.go
Normal file
|
|
@ -0,0 +1,374 @@
|
|||
//go:build cgo
|
||||
|
||||
package backend
|
||||
|
||||
import (
|
||||
"os"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// reconnect_test.go exercises reconnect.go's rejoin flag-setter,
|
||||
// rejoin-performing function, and shared post-call-check helper
|
||||
// (component-failover-adoption, task 4.3, boundary: ApiserverBackendBridge,
|
||||
// Req 2 AC1-3/AC5). Kept //go:build cgo since these symbols only exist in
|
||||
// the CGO build -- this file itself does not `import "C"` (a _test.go file
|
||||
// can never do that directly, golang/go#13938): it only calls the
|
||||
// plain-Go-signature functions defined in reconnect.go (setNeedsRejoin,
|
||||
// checkAndPerformRejoin, performRejoinFn), the same clib.go/clib_test.go
|
||||
// and candidates.go/candidates_test.go split already established in this
|
||||
// package.
|
||||
//
|
||||
// Test strategy:
|
||||
// - The flag-setter's "no I/O, returns immediately" contract (Req 2 AC1,
|
||||
// design.md's "Rejoin callback constraint" -- the library invokes it
|
||||
// while still holding ac->failover.lock, so any I/O or xcecli_* call
|
||||
// there self-deadlocks) is proven two ways, mirroring
|
||||
// test_scheduler_connect.c's identical reasoning for the C-side
|
||||
// rejoin callback (see that file's own header comment): a
|
||||
// source-text scan confirming go_backend_rejoin_handler's body makes
|
||||
// no xcecli_*/blocking call, and a timing-based fast-return assertion
|
||||
// on setNeedsRejoin (the plain-Go-typed logic go_backend_rejoin_handler
|
||||
// delegates to -- go_backend_rejoin_handler itself can't be called
|
||||
// directly from this file since its signature involves C.int).
|
||||
// - Retry-on-failure (Req 2 AC5) is exercised by overriding
|
||||
// performRejoinFn with a scripted double across two separate
|
||||
// checkAndPerformRejoin() calls (simulating two separate
|
||||
// backend-call-wrapper invocations) -- no live controller or linker
|
||||
// interposition needed, mirrors worker's rejoin_call function-pointer
|
||||
// testability pattern (controller_connector.c).
|
||||
// - The "correctly-placed cgo registration" structural claim (this
|
||||
// task's own title) is a source-text scan proving the C shim/extern/
|
||||
// registration all live in clib.go, not reconnect.go, and that
|
||||
// registration happens after xcecli_start()'s success check -- same
|
||||
// genre as clib_test.go's own TestClibSource_* structural regression
|
||||
// tests.
|
||||
|
||||
func resetNeedsRejoinForTest() {
|
||||
atomic.StoreInt32(&needsRejoin, 0)
|
||||
}
|
||||
|
||||
func readSourceForTest(t *testing.T, filename string) string {
|
||||
t.Helper()
|
||||
src, err := os.ReadFile(filename)
|
||||
if err != nil {
|
||||
t.Fatalf("could not read %s source for scan: %v", filename, err)
|
||||
}
|
||||
return string(src)
|
||||
}
|
||||
|
||||
// stripGoLineComments removes everything from the first "//" to end-of-line
|
||||
// on every line, for source-text scans that must not be tripped by a
|
||||
// doc comment merely mentioning a symbol name in prose. Deliberately
|
||||
// simplistic (no string-literal awareness) -- sufficient for scanning this
|
||||
// package's own source, which contains no "//" inside a string literal on
|
||||
// a line this test cares about.
|
||||
func stripGoLineComments(src string) string {
|
||||
lines := strings.Split(src, "\n")
|
||||
for i, line := range lines {
|
||||
if idx := strings.Index(line, "//"); idx >= 0 {
|
||||
lines[i] = line[:idx]
|
||||
}
|
||||
}
|
||||
return strings.Join(lines, "\n")
|
||||
}
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// Req 2 AC1 -- flag-setter performs zero I/O and returns immediately, even
|
||||
// conceptually "while the client's lock is held"
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
func TestSetNeedsRejoin_FastReturnAndSetsFlag(t *testing.T) {
|
||||
resetNeedsRejoinForTest()
|
||||
|
||||
start := time.Now()
|
||||
setNeedsRejoin()
|
||||
elapsed := time.Since(start)
|
||||
|
||||
// Generous bound for a flag-only, no-I/O operation -- anything even
|
||||
// approaching millisecond scale would suggest a blocking call snuck in.
|
||||
if elapsed > 10*time.Millisecond {
|
||||
t.Errorf("setNeedsRejoin took %v -- too slow for a flag-only "+
|
||||
"operation, suggests I/O or blocking was introduced", elapsed)
|
||||
}
|
||||
if atomic.LoadInt32(&needsRejoin) != 1 {
|
||||
t.Error("expected needsRejoin to be set after setNeedsRejoin()")
|
||||
}
|
||||
}
|
||||
|
||||
func TestReconnectSource_FlagSetterBodyHasNoXcecliCallsOrIO(t *testing.T) {
|
||||
src := readSourceForTest(t, "reconnect.go")
|
||||
|
||||
start := strings.Index(src, "func go_backend_rejoin_handler(")
|
||||
if start < 0 {
|
||||
t.Fatal("go_backend_rejoin_handler not found in reconnect.go")
|
||||
}
|
||||
end := strings.Index(src[start:], "\n}\n")
|
||||
if end < 0 {
|
||||
t.Fatal("could not find end of go_backend_rejoin_handler's body")
|
||||
}
|
||||
body := src[start : start+end]
|
||||
|
||||
for _, forbidden := range []string{"C.xcecli_", "net.Dial", "os.Open", "time.Sleep", "<-"} {
|
||||
if strings.Contains(body, forbidden) {
|
||||
t.Errorf("go_backend_rejoin_handler body contains %q -- the "+
|
||||
"library invokes this callback while still holding "+
|
||||
"ac->failover.lock (client_failover.c:341-411); any I/O "+
|
||||
"or xcecli_* call here self-deadlocks", forbidden)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// Req 2 AC5 -- rejoin retry-on-failure: retried on the NEXT backend call,
|
||||
// never abandoned
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
func TestCheckAndPerformRejoin_NoPendingRejoin_DoesNothing(t *testing.T) {
|
||||
resetNeedsRejoinForTest()
|
||||
orig := performRejoinFn
|
||||
defer func() { performRejoinFn = orig }()
|
||||
|
||||
called := false
|
||||
performRejoinFn = func() int {
|
||||
called = true
|
||||
return 0
|
||||
}
|
||||
|
||||
checkAndPerformRejoin()
|
||||
|
||||
if called {
|
||||
t.Error("performRejoinFn must not be called when no rejoin is pending")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckAndPerformRejoin_PendingRejoinSucceeds_ClearsFlag(t *testing.T) {
|
||||
captureLog(t) // checkAndPerformRejoin logs via utils.Log; nil otherwise
|
||||
atomic.StoreInt32(&needsRejoin, 1)
|
||||
orig := performRejoinFn
|
||||
defer func() { performRejoinFn = orig }()
|
||||
|
||||
performRejoinFn = func() int { return 0 } // rc >= 0 => success
|
||||
|
||||
checkAndPerformRejoin()
|
||||
|
||||
if atomic.LoadInt32(&needsRejoin) != 0 {
|
||||
t.Error("expected needsRejoin to clear after a successful rejoin")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckAndPerformRejoin_NonZeroSuccessStillClearsFlag(t *testing.T) {
|
||||
// Matches worker's/scheduler's established convention: a successful
|
||||
// join can return a non-negative assigned slot/identity value, not
|
||||
// always exactly 0 -- rc < 0 is the only correct failure test.
|
||||
captureLog(t)
|
||||
atomic.StoreInt32(&needsRejoin, 1)
|
||||
orig := performRejoinFn
|
||||
defer func() { performRejoinFn = orig }()
|
||||
|
||||
performRejoinFn = func() int { return 7 }
|
||||
|
||||
checkAndPerformRejoin()
|
||||
|
||||
if atomic.LoadInt32(&needsRejoin) != 0 {
|
||||
t.Error("expected needsRejoin to clear on a non-zero-but-non-negative rc")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckAndPerformRejoin_RetriedOnNextBackendCall_NotAbandoned(t *testing.T) {
|
||||
captureLog(t)
|
||||
atomic.StoreInt32(&needsRejoin, 1)
|
||||
orig := performRejoinFn
|
||||
defer func() { performRejoinFn = orig }()
|
||||
|
||||
calls := 0
|
||||
performRejoinFn = func() int {
|
||||
calls++
|
||||
if calls == 1 {
|
||||
return -1 // simulates -E_XCE_NOT_READY: this attempt's own retry budget exhausted
|
||||
}
|
||||
return 3 // succeeds on the second, separate attempt
|
||||
}
|
||||
|
||||
// First "backend call" wrapper invocation: rejoin attempted, fails --
|
||||
// the flag must stay set, not be cleared merely because it was
|
||||
// attempted.
|
||||
checkAndPerformRejoin()
|
||||
if atomic.LoadInt32(&needsRejoin) != 1 {
|
||||
t.Fatal("expected needsRejoin to remain set after a failed rejoin attempt")
|
||||
}
|
||||
if calls != 1 {
|
||||
t.Fatalf("expected exactly 1 rejoin attempt so far, got %d", calls)
|
||||
}
|
||||
|
||||
// Second, separate "backend call" wrapper invocation (simulating the
|
||||
// NEXT backend call, not a retry loop inside the first): rejoin is
|
||||
// retried and succeeds, flag clears.
|
||||
checkAndPerformRejoin()
|
||||
if atomic.LoadInt32(&needsRejoin) != 0 {
|
||||
t.Error("expected needsRejoin to clear once the retried rejoin succeeds")
|
||||
}
|
||||
if calls != 2 {
|
||||
t.Errorf("expected exactly 2 rejoin attempts total (one per backend call), got %d", calls)
|
||||
}
|
||||
}
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// This task's own title -- correctly-placed cgo registration
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
func TestClibSource_RejoinShimExternAndRegistrationLiveInClibGo(t *testing.T) {
|
||||
clibSrc := readSourceForTest(t, "clib.go")
|
||||
|
||||
if !strings.Contains(clibSrc, "extern int go_backend_rejoin_handler(") {
|
||||
t.Error("clib.go must declare `extern int go_backend_rejoin_handler(...)` " +
|
||||
"in its own cgo preamble -- cgo symbol resolution is per-file, " +
|
||||
"so a C.<name> reference (here, the shim calling the Go export) " +
|
||||
"only resolves against an extern declared in the SAME file")
|
||||
}
|
||||
if !strings.Contains(clibSrc, "static int xce_backend_rejoin_handler(") {
|
||||
t.Error("clib.go must define the C shim `xce_backend_rejoin_handler` " +
|
||||
"that wraps go_backend_rejoin_handler, mirroring the existing " +
|
||||
"xce_backend_handler/go_push_handler pattern")
|
||||
}
|
||||
if !strings.Contains(clibSrc, "xcecli_set_reconnect_handler(") {
|
||||
t.Error("clib.go must call xcecli_set_reconnect_handler(...) to " +
|
||||
"register the rejoin shim with the client library")
|
||||
}
|
||||
|
||||
// Code-only view of reconnect.go (comment lines stripped): reconnect.go's
|
||||
// own doc comments legitimately MENTION xce_backend_rejoin_handler and
|
||||
// xcecli_set_reconnect_handler in prose (explaining where they live and
|
||||
// why) -- that must not trip this check. Only actual C code (the shim
|
||||
// definition or the registration call) is disallowed here.
|
||||
reconnectCode := stripGoLineComments(readSourceForTest(t, "reconnect.go"))
|
||||
if strings.Contains(reconnectCode, "xce_backend_rejoin_handler") {
|
||||
t.Error("the C shim (xce_backend_rejoin_handler) must live in " +
|
||||
"clib.go, not reconnect.go -- cgo symbol resolution is per-file")
|
||||
}
|
||||
if strings.Contains(reconnectCode, "xcecli_set_reconnect_handler(") {
|
||||
t.Error("the xcecli_set_reconnect_handler(...) registration call " +
|
||||
"must live in clib.go, not reconnect.go")
|
||||
}
|
||||
|
||||
// Ordering: registration happens AFTER xcecli_start()'s success check,
|
||||
// not before -- xcecli_start() (client.c) unconditionally resets
|
||||
// ctx->failover.rejoin_cb to NULL as part of initializing per-context
|
||||
// failover state, so registering before it would be silently wiped
|
||||
// (tasks.md Implementation Notes, task 2.3's identical worker bug).
|
||||
startIdx := strings.Index(clibSrc, "rc = xcecli_start(ctx->ctx);")
|
||||
registerIdx := strings.Index(clibSrc, "xcecli_set_reconnect_handler(ctx->ctx")
|
||||
if startIdx < 0 {
|
||||
t.Fatal("expected `rc = xcecli_start(ctx->ctx);` call site not found in clib.go")
|
||||
}
|
||||
if registerIdx < 0 {
|
||||
t.Fatal("expected xcecli_set_reconnect_handler(ctx->ctx, ...) call site not found in clib.go")
|
||||
}
|
||||
if registerIdx < startIdx {
|
||||
t.Error("xcecli_set_reconnect_handler must be registered AFTER " +
|
||||
"xcecli_start(), not before -- xcecli_start() wipes any " +
|
||||
"handler registered earlier")
|
||||
}
|
||||
}
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// Shared helper wraps every backend call: structural check that
|
||||
// checkAndPerformRejoin is invoked from exactly one place in clib.go.
|
||||
//
|
||||
// Superseded by task 4.5's consolidation (component-failover-adoption,
|
||||
// boundary: ApiserverBackendBridge/ApiserverBackendHealth): task 4.3
|
||||
// originally wired checkAndPerformRejoin() into each of
|
||||
// SubmitJob/ControlJob/ControlQueue/ControlHost directly (>=4 literal call
|
||||
// sites, this test's original assertion). Task 4.5 folds that post-call
|
||||
// check into the single shared callBackend helper (also housing the new
|
||||
// pre-call IsDegraded() check), so checkAndPerformRejoin() now appears
|
||||
// exactly ONCE in clib.go's source -- inside callBackend -- with every
|
||||
// wrapper routing through callBackend instead
|
||||
// (TestClibSource_BackendCallWrappersCallSharedCallBackendHelper,
|
||||
// health_rejoin_test.go, is this test's direct successor for "wrappers
|
||||
// don't duplicate the shared logic"). This test now only guards against a
|
||||
// regression back to per-wrapper duplication of the rejoin check itself.
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
func TestClibSource_BackendCallWrappersCallSharedRejoinHelper(t *testing.T) {
|
||||
clibSrc := readSourceForTest(t, "clib.go")
|
||||
|
||||
count := strings.Count(clibSrc, "checkAndPerformRejoin()")
|
||||
if count != 1 {
|
||||
t.Errorf("expected exactly 1 call site of checkAndPerformRejoin() in "+
|
||||
"clib.go (inside the shared callBackend helper, task 4.5), got %d "+
|
||||
"-- either it has been duplicated back out to individual wrappers, "+
|
||||
"or callBackend itself no longer calls it", count)
|
||||
}
|
||||
}
|
||||
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
// Concurrent-request safety (component-failover-adoption task 4.6,
|
||||
// boundary: ApiserverBackendBridge). health_test.go already stress-tests
|
||||
// health.go's degraded-until state under concurrent goroutines
|
||||
// (TestHealthState_ConcurrentReadersAndWriters_NoDataCorruption), but that
|
||||
// test never touches needsRejoin/checkAndPerformRejoin at all -- and unlike
|
||||
// a plain atomic load/store pair, checkAndPerformRejoin() is a genuine
|
||||
// check-then-act compound operation (load needsRejoin, conditionally call
|
||||
// performRejoinFn, conditionally store) reached from real concurrent
|
||||
// production call sites: all 5 of clib.go's callBackend-wrapped functions
|
||||
// (SubmitJob/ControlJob/ControlQueue/ControlHost/FetchFile) run on
|
||||
// per-HTTP-request goroutines, plus heartbeat.go's own goroutine calls it on
|
||||
// every successful tick (task 4.4) -- so multiple goroutines can genuinely
|
||||
// call checkAndPerformRejoin() at the same moment after a failover, exactly
|
||||
// as task 4.3's own Implementation Note already anticipated ("concurrent
|
||||
// goroutines can each observe needsRejoin==1 ... and each call the real
|
||||
// xcecli_join()"). That note explains why this is correctness-safe in
|
||||
// production (serialized by the C library's own per-ctx lock) but it was
|
||||
// never backed by a test proving the Go-level state machine itself (flag
|
||||
// clear/no-clear, no panic) holds up under real concurrent access -- this
|
||||
// closes that gap. Deliberately does NOT assert exactly-one call to
|
||||
// performRejoinFn: redundant concurrent calls are the documented, accepted
|
||||
// behavior (a CAS-based single-flight guard was explicitly judged a
|
||||
// reasonable-but-not-required fast-follow, not a bug), so asserting
|
||||
// call-count==1 would encode a guarantee the design never made.
|
||||
// ════════════════════════════════════════════════════════════════════════
|
||||
|
||||
func TestCheckAndPerformRejoin_ConcurrentCallers_NoDataCorruptionAndFlagClears(t *testing.T) {
|
||||
captureLog(t)
|
||||
resetNeedsRejoinForTest()
|
||||
defer resetNeedsRejoinForTest()
|
||||
|
||||
orig := performRejoinFn
|
||||
defer func() { performRejoinFn = orig }()
|
||||
|
||||
var joinCalls int32
|
||||
performRejoinFn = func() int {
|
||||
atomic.AddInt32(&joinCalls, 1)
|
||||
return 0 // always succeeds
|
||||
}
|
||||
|
||||
atomic.StoreInt32(&needsRejoin, 1)
|
||||
|
||||
const goroutines = 50
|
||||
var wg sync.WaitGroup
|
||||
wg.Add(goroutines)
|
||||
for i := 0; i < goroutines; i++ {
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
checkAndPerformRejoin()
|
||||
}()
|
||||
}
|
||||
wg.Wait()
|
||||
// Reaching here without a panic/deadlock, under concurrent goroutines
|
||||
// each racing through the load-call-store sequence, is itself part of
|
||||
// what this test proves.
|
||||
|
||||
if got := atomic.LoadInt32(&joinCalls); got < 1 {
|
||||
t.Errorf("expected performRejoinFn to be called at least once across "+
|
||||
"%d concurrent checkAndPerformRejoin() callers, got %d", goroutines, got)
|
||||
}
|
||||
if atomic.LoadInt32(&needsRejoin) != 0 {
|
||||
t.Error("expected needsRejoin to end up cleared: every concurrent " +
|
||||
"performRejoinFn call in this test succeeds (rc=0), so whichever " +
|
||||
"goroutine's store lands last still clears the flag to 0")
|
||||
}
|
||||
}
|
||||
|
|
@ -14,6 +14,7 @@ import (
|
|||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"strings"
|
||||
|
||||
"apiserver/controllers"
|
||||
|
||||
|
|
@ -21,12 +22,55 @@ import (
|
|||
"go.uber.org/zap"
|
||||
)
|
||||
|
||||
// GitCommit and BuildTime are injected at link time via
|
||||
// `-ldflags "-X main.GitCommit=... -X main.BuildTime=..."` (see
|
||||
// src/tools/go_build and src/frontend/Makefile's build-server target). The
|
||||
// "unknown" defaults matter: a bare `go build`/`go run` that bypasses that
|
||||
// ldflags injection must still report "unknown" rather than an empty
|
||||
// string, matching this feature's unknown-fallback requirement (see
|
||||
// xce_version_field() in src/lib/utils.c for the equivalent C-side
|
||||
// convention).
|
||||
//
|
||||
// DirtyFingerprint is injected the same way (`-X main.DirtyFingerprint=...`,
|
||||
// see src/tools/go_build task 4.1) but its default is a deliberately EMPTY
|
||||
// string, not "unknown" -- matching dirty_fingerprint's convention
|
||||
// everywhere else in the dirty-content-fingerprint feature (empty means
|
||||
// "no fingerprint available", distinct from GitCommit/BuildTime's
|
||||
// "unknown" sentinel for "not injected at all").
|
||||
var (
|
||||
GitCommit = "unknown"
|
||||
BuildTime = "unknown"
|
||||
DirtyFingerprint = ""
|
||||
)
|
||||
|
||||
// composeVersionDisplay applies the dirty-content-fingerprint feature's
|
||||
// strip-and-append rule to gitCommit for --version/-version display only:
|
||||
// if dirtyFingerprint is non-empty, a trailing "-dirty" suffix on gitCommit
|
||||
// is stripped (an anchored suffix check via strings.TrimSuffix, not a blind
|
||||
// substring replace) and "-<dirtyFingerprint>" is appended in its place;
|
||||
// otherwise gitCommit is returned exactly as-is (already correctly showing
|
||||
// "-dirty" or not, per the original build-time-versioning feature). Mirrors
|
||||
// xce_compose_version_ex() in src/lib/utils.c (task 3.1), the equivalent
|
||||
// C-side implementation already approved for controller/scheduler/worker.
|
||||
//
|
||||
// Must be applied ONLY to the -version flag's printed output below -- never
|
||||
// to the logger.Info("********** Initialize Logger done!") startup log
|
||||
// call, which must keep logging the raw GitCommit var unchanged
|
||||
// (Requirement 3.2).
|
||||
func composeVersionDisplay(gitCommit, dirtyFingerprint string) string {
|
||||
if dirtyFingerprint == "" {
|
||||
return gitCommit
|
||||
}
|
||||
return strings.TrimSuffix(gitCommit, "-dirty") + "-" + dirtyFingerprint
|
||||
}
|
||||
|
||||
func main() {
|
||||
gin.SetMode(gin.ReleaseMode)
|
||||
gin.DefaultWriter = io.Discard
|
||||
gin.DefaultErrorWriter = io.Discard
|
||||
|
||||
topDir := flag.String("d", "", "Top directory (default: current directory)")
|
||||
versionFlag := flag.Bool("version", false, "Print version information and exit")
|
||||
flag.Usage = func() {
|
||||
fmt.Fprintln(os.Stderr, `
|
||||
Usage: apiserver -d <top_directory>
|
||||
|
|
@ -35,12 +79,22 @@ func main() {
|
|||
}
|
||||
flag.Parse()
|
||||
|
||||
// Checked before utils.InitLogger() runs so -version has no logging
|
||||
// side effects: it must only print and exit, matching controller's/
|
||||
// scheduler's/worker's own --version contract (Requirement 2.1).
|
||||
if *versionFlag {
|
||||
fmt.Printf("xce-apiserver %s built %s\n", composeVersionDisplay(GitCommit, DirtyFingerprint), BuildTime)
|
||||
return
|
||||
}
|
||||
|
||||
utils.Global_TopDir = utils.PromptForTopDir(topDir)
|
||||
|
||||
if err := utils.InitLogger(); err != nil {
|
||||
os.Exit(1)
|
||||
}
|
||||
logger.Info("********** Initialize Logger done!")
|
||||
logger.Info("********** Initialize Logger done!",
|
||||
zap.String("gitCommit", GitCommit),
|
||||
zap.String("buildTime", BuildTime))
|
||||
|
||||
if err := utils.InitConf(); err != nil {
|
||||
logger.Error("Failed to initialize configuration files", zap.Error(err))
|
||||
|
|
|
|||
64
src/frontend/apiserver/cmd/main_test.go
Normal file
64
src/frontend/apiserver/cmd/main_test.go
Normal file
|
|
@ -0,0 +1,64 @@
|
|||
package main
|
||||
|
||||
import "testing"
|
||||
|
||||
// dirty-content-fingerprint task 4.4: composeVersionDisplay implements the
|
||||
// --version-only strip-and-append rule mirroring xce_compose_version_ex()
|
||||
// in src/lib/utils.c (task 3.1). It must never be reachable from the
|
||||
// logger.Info("********** Initialize Logger done!") startup log call in
|
||||
// main(), which always logs the raw GitCommit var unchanged.
|
||||
func TestComposeVersionDisplay(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
gitCommit string
|
||||
dirtyFingerprint string
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "dirty commit with fingerprint strips -dirty and appends fingerprint",
|
||||
gitCommit: "abc1234-dirty",
|
||||
dirtyFingerprint: "deadbeef",
|
||||
want: "abc1234-deadbeef",
|
||||
},
|
||||
{
|
||||
name: "clean commit with no fingerprint is unchanged",
|
||||
gitCommit: "abc1234",
|
||||
dirtyFingerprint: "",
|
||||
want: "abc1234",
|
||||
},
|
||||
{
|
||||
name: "dirty commit with no fingerprint (degraded) is unchanged, -dirty preserved",
|
||||
gitCommit: "abc1234-dirty",
|
||||
dirtyFingerprint: "",
|
||||
want: "abc1234-dirty",
|
||||
},
|
||||
{
|
||||
name: "unknown commit with no fingerprint is unchanged",
|
||||
gitCommit: "unknown",
|
||||
dirtyFingerprint: "",
|
||||
want: "unknown",
|
||||
},
|
||||
{
|
||||
name: "clean commit with a fingerprint still appends (nothing to strip)",
|
||||
gitCommit: "abc1234",
|
||||
dirtyFingerprint: "cafef00d",
|
||||
want: "abc1234-cafef00d",
|
||||
},
|
||||
{
|
||||
name: "suffix check is anchored, not a blind substring removal",
|
||||
gitCommit: "abc1234-dirty-not-at-end-dirty",
|
||||
dirtyFingerprint: "cafef00d",
|
||||
want: "abc1234-dirty-not-at-end-cafef00d",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := composeVersionDisplay(tt.gitCommit, tt.dirtyFingerprint)
|
||||
if got != tt.want {
|
||||
t.Errorf("composeVersionDisplay(%q, %q) = %q, want %q",
|
||||
tt.gitCommit, tt.dirtyFingerprint, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -550,7 +550,7 @@ func (j JobController) GetJobsInfo(ctx *gin.Context) {
|
|||
found2 := false
|
||||
found3 := false
|
||||
// verify the fields is in supported list
|
||||
supportedFields := utils.GetSupportedFields(models.JobSubmitRequest{})
|
||||
supportedFields := utils.GetSupportedFields(models.JobInfo{})
|
||||
|
||||
utils.AddFieldsToSupportedList(&supportedFields, models.JobAdditionalFields{})
|
||||
logger.Debug("job supportedFields: %v", zap.Any("supportedFields", supportedFields))
|
||||
|
|
@ -898,7 +898,7 @@ func (j JobController) GetJobByIDs(ctx *gin.Context) {
|
|||
found1 := false
|
||||
found2 := false
|
||||
// verify the fields is in supported list
|
||||
supportedFields := utils.GetSupportedFields(models.JobSubmitRequest{})
|
||||
supportedFields := utils.GetSupportedFields(models.JobInfo{})
|
||||
utils.AddFieldsToSupportedList(&supportedFields, models.JobAdditionalFields{})
|
||||
logger.Debug("job supportedFields: %v", zap.Any("supportedFields", supportedFields))
|
||||
|
||||
|
|
@ -1480,7 +1480,7 @@ func (j JobController) CreateJob(ctx *gin.Context) {
|
|||
UserPriority: req.JobPriority,
|
||||
DependCond: req.DependCond,
|
||||
CWD: req.CWD,
|
||||
FromHost: ctx.ClientIP(), //use ip now, need use hostname ?
|
||||
FromHost: req.FromHost, // bug-1676: client-asserted hostname, passed through unchanged
|
||||
PreExecCmd: req.PreExecCmd,
|
||||
PostExecCmd: req.PostExecCmd,
|
||||
RequeueErrorValues: req.RequeueErrorValues,
|
||||
|
|
|
|||
|
|
@ -36,6 +36,51 @@ func TestPreCheckJobRequest_LostAndFound(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// TestJobsListFieldValidation_UsesJobInfoNotSubmitRequest is a regression
|
||||
// guard for bug-1674 (gitea #41): GetJobsInfo (jobs.go:553) and GetJobByIDs
|
||||
// (jobs.go:901) validate a caller's requested `-o`/`-l` field list against
|
||||
// utils.GetSupportedFields(models.JobInfo{}) merged with
|
||||
// models.JobAdditionalFields{} -- not models.JobSubmitRequest{}, the
|
||||
// job-submission model. Response-only fields like pending_reason/start_time/
|
||||
// end_time exist on JobInfo but not on JobSubmitRequest; validating against
|
||||
// the wrong struct made `-l` (which requests these fields) fail outright with
|
||||
// "unsupported query field" on every invocation, regardless of which such
|
||||
// field Go's randomized map iteration hit first.
|
||||
//
|
||||
// allXcecliJobFields mirrors xcecli/models/jobs.go's JobFieldsMap value set
|
||||
// verbatim (xcecli is a separate Go module -- see src/frontend/CLAUDE.md --
|
||||
// so it cannot be imported directly here). Every value in that map is a
|
||||
// field xcecli's `-o`/`-l` can actually send the apiserver; this test proves
|
||||
// the combined JobInfo+JobAdditionalFields supported-fields list -- what
|
||||
// both fixed call sites now validate against -- covers every one of them.
|
||||
var allXcecliJobFields = []string{
|
||||
"job_id", "submit_user", "job_status", "queue_name", "command",
|
||||
"job_name", "submit_time", "from_host", "exec_hosts", "core_limit",
|
||||
"mem_limit", "pids", "pgids", "project_name", "start_time", "end_time",
|
||||
"cwd", "cpu_time", "resreq", "job_description", "launch_type",
|
||||
"num_processors", "interactive_job_url", "pending_reason",
|
||||
}
|
||||
|
||||
func TestJobsListFieldValidation_UsesJobInfoNotSubmitRequest(t *testing.T) {
|
||||
supportedFields := utils.GetSupportedFields(models.JobInfo{})
|
||||
utils.AddFieldsToSupportedList(&supportedFields, models.JobAdditionalFields{})
|
||||
|
||||
for _, field := range allXcecliJobFields {
|
||||
if !utils.ContainsField(supportedFields, field) {
|
||||
t.Errorf("GetSupportedFields(models.JobInfo{}) + JobAdditionalFields is missing %q -- xcecli's `-o`/`-l` can request this field (see xcecli/models/jobs.go's JobFieldsMap) and it must validate successfully", field)
|
||||
}
|
||||
}
|
||||
|
||||
// Documents exactly why validating against JobSubmitRequest broke:
|
||||
// response-only fields like pending_reason don't exist there at all.
|
||||
submitRequestFields := utils.GetSupportedFields(models.JobSubmitRequest{})
|
||||
for _, field := range []string{"pending_reason", "start_time", "end_time"} {
|
||||
if utils.ContainsField(submitRequestFields, field) {
|
||||
t.Errorf("unexpected: models.JobSubmitRequest{} contains %q -- this test's premise (that JobSubmitRequest lacks response-only fields) no longer holds", field)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestPreCheckJobRequest_NormalQueue verifies that a normal (non-reserved)
|
||||
// queue name does not trigger the reserved-name rejection.
|
||||
func TestPreCheckJobRequest_NormalQueue(t *testing.T) {
|
||||
|
|
|
|||
|
|
@ -56,6 +56,9 @@ custom_target('apiserver',
|
|||
command: [env_program,
|
||||
'CURRENT_SOURCE_DIR="@CURRENT_SOURCE_DIR@"',
|
||||
'BUILD_ROOT="@BUILD_ROOT@"',
|
||||
'XCE_GIT_COMMIT=' + get_option('commit_id'),
|
||||
'XCE_BUILD_TIME=' + get_option('build_time'),
|
||||
'XCE_DIRTY_FINGERPRINT=' + get_option('dirty_fingerprint'),
|
||||
go_build_program,
|
||||
'@INPUT@', '@OUTPUT@'],
|
||||
env: go_env,
|
||||
|
|
|
|||
|
|
@ -206,7 +206,6 @@ var JOB_DEFAULT_COLUMN = []string{
|
|||
"job_name",
|
||||
"command",
|
||||
"submit_time",
|
||||
"pending_reason",
|
||||
}
|
||||
|
||||
var ARRAY_JOB_DEFAULT_COLUMN = []string{
|
||||
|
|
@ -613,7 +612,13 @@ type JobAdditionalFields struct {
|
|||
Pids []int `json:"pids"` // List of process IDs associated with the job
|
||||
Pgids []int `json:"pgids"` // List of process group IDs associated with the job
|
||||
InteractiveJobURL string `json:"interactive_job_url"`
|
||||
PendingReason Jsonb `json:"pending_reason"` // PEND reason (scheduler-set, JSONB object)
|
||||
// CoreLimit/MemLimit: JobInfo has no equivalent field (these are
|
||||
// submission-time resource limits, not job-state fields), but xcecli's
|
||||
// JobFieldsMap accepts them as valid -o/-l display fields (bug-1674) --
|
||||
// merged in here rather than validating against JobSubmitRequest itself,
|
||||
// which lacks response-only fields like pending_reason.
|
||||
CoreLimit CustomLimit `json:"core_limit"`
|
||||
MemLimit CustomLimit `json:"mem_limit"`
|
||||
}
|
||||
|
||||
type JobSubmitRequest struct {
|
||||
|
|
@ -657,6 +662,7 @@ type JobSubmitRequest struct {
|
|||
ChkpntDir string `json:"chkpnt_dir,omitempty"`
|
||||
InitChkpntPeriod int `json:"init_chkpnt_period,omitempty"`
|
||||
CWD string `json:"cwd,omitempty"`
|
||||
FromHost string `json:"from_host,omitempty"` // client-asserted submitting hostname (bug-1676) -- passed through unchanged, not derived server-side
|
||||
OutDir string `json:"out_dir,omitempty"`
|
||||
InFile string `json:"in_file,omitempty"`
|
||||
OutFile string `json:"out_file,omitempty"`
|
||||
|
|
@ -697,7 +703,6 @@ type ShellLimit struct {
|
|||
|
||||
type JobSubmitResponse struct {
|
||||
JobID int64 `json:"jobid"`
|
||||
JobIDHuman string `json:"job_id_human"` // cluster-apiserver-timestamp-seq, for xce_test/xcecli cross-reference
|
||||
QueueName string `json:"queue_name"`
|
||||
Message string `json:"message"`
|
||||
}
|
||||
|
|
|
|||
|
|
@ -17,16 +17,10 @@ func ExecuteQuery(rows []utils.OrderMap, opts *utils.QueryOptions, fields []stri
|
|||
return SelectFields(rows, fields), ""
|
||||
}
|
||||
|
||||
// 1. Filter — both the flat AND-conditions AND the compound (per-field OR,
|
||||
// array-aware) conditions. bug-1490-adjacent: CompConditions used to be
|
||||
// silently dropped here, so GET /xce/v1/jobs/:jobids returned the whole
|
||||
// job table (the mcp scheduler_get_job tool never saw the requested job).
|
||||
// 1. Filter
|
||||
if len(opts.Conditions) > 0 {
|
||||
rows = FilterRows(rows, opts.Conditions)
|
||||
}
|
||||
if len(opts.CompConditions) > 0 {
|
||||
rows = FilterCompConditions(rows, opts.CompConditions)
|
||||
}
|
||||
|
||||
// 2. Sort
|
||||
if len(opts.Orders) > 0 {
|
||||
|
|
@ -84,18 +78,8 @@ func MatchesCondition(row utils.OrderMap, cond utils.Condition) bool {
|
|||
// MatchesBitmaskCondition handles bitmask fields (job_status, host_status).
|
||||
// For bitmask EQ: (val & mask) != 0 means the status is set.
|
||||
// For bitmask NE: (val & mask) == 0 means the status is NOT set.
|
||||
// null/notnull are handled on the raw field value (bug-1504: job_status
|
||||
// notnull previously fell through to `return false`, so `jobs list -a`
|
||||
// filtered out every job).
|
||||
func MatchesBitmaskCondition(row utils.OrderMap, field string, bitmask int64, op utils.OperatorType) bool {
|
||||
fieldVal := row.Data[field]
|
||||
switch op {
|
||||
case utils.OpISNULL:
|
||||
return fieldVal == nil
|
||||
case utils.OpNOTNULL:
|
||||
return fieldVal != nil
|
||||
}
|
||||
val := toInt64(fieldVal)
|
||||
val := toInt64(row.Data[field])
|
||||
result := val & bitmask
|
||||
switch op {
|
||||
case utils.OpNE: // bitmask "eq" is stored as OpNE (val & mask != 0)
|
||||
|
|
@ -111,75 +95,36 @@ func IsBitmaskField(field string) bool {
|
|||
return field == "job_status" || field == "host_status"
|
||||
}
|
||||
|
||||
// rowMatchesCondition evaluates a single condition against a row, dispatching
|
||||
// bitmask fields (job_status/host_status) to MatchesBitmaskCondition and all
|
||||
// others to MatchesCondition.
|
||||
func rowMatchesCondition(row utils.OrderMap, cond utils.Condition) bool {
|
||||
if IsBitmaskField(cond.Column) {
|
||||
bitmask, ok := cond.Value.Value.(int64)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
return MatchesBitmaskCondition(row, cond.Column, bitmask, cond.Operator)
|
||||
}
|
||||
return MatchesCondition(row, cond)
|
||||
}
|
||||
|
||||
// FilterRows applies all conditions (AND logic) to rows.
|
||||
func FilterRows(rows []utils.OrderMap, conditions []utils.Condition) []utils.OrderMap {
|
||||
result := make([]utils.OrderMap, 0, len(rows))
|
||||
for _, row := range rows {
|
||||
match := true
|
||||
for _, cond := range conditions {
|
||||
if !rowMatchesCondition(row, cond) {
|
||||
match = false
|
||||
break
|
||||
}
|
||||
}
|
||||
if match {
|
||||
result = append(result, row)
|
||||
}
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
// FilterCompConditions applies compound conditions — the per-field OR structure
|
||||
// built by ConvertToNewFilterArray for array-aware queries (e.g. job_id with
|
||||
// array_idx sub-ranges in GET /xce/v1/jobs/:jobids, or job_name filters in
|
||||
// GET /xce/v1/jobs). A row must match EVERY field key present (AND across
|
||||
// fields); within a field, the row matches if ANY of its ConditionArrays
|
||||
// matches. A ConditionArray matches when its MainCondition holds AND either it
|
||||
// has no SubORConditions or any sub-condition holds.
|
||||
func FilterCompConditions(rows []utils.OrderMap, comp utils.CompCondition) []utils.OrderMap {
|
||||
if len(comp) == 0 {
|
||||
return rows
|
||||
}
|
||||
result := make([]utils.OrderMap, 0, len(rows))
|
||||
for _, row := range rows {
|
||||
match := true
|
||||
for _, arrays := range comp {
|
||||
fieldMatch := false
|
||||
for _, arr := range arrays {
|
||||
if !rowMatchesCondition(row, arr.MainCondition) {
|
||||
continue
|
||||
}
|
||||
if len(arr.SubORConditions) == 0 {
|
||||
fieldMatch = true
|
||||
// bug-1680: IsBitmaskField alone is not enough to route into the
|
||||
// bitmask path -- OpEQ/OpNE are the only two operator shapes
|
||||
// buildIntCondition's in/notin bitmask branch ever produces.
|
||||
// Any other operator on job_status/host_status (e.g. OpNOTNULL,
|
||||
// from xcecli's -a flag building `{job_status:notnull:}`) must
|
||||
// fall through to the generic evaluator below, which already
|
||||
// handles it correctly -- MatchesBitmaskCondition's own switch
|
||||
// only knows OpEQ/OpNE and silently returns false (never
|
||||
// matching) for anything else.
|
||||
if IsBitmaskField(cond.Column) && (cond.Operator == utils.OpEQ || cond.Operator == utils.OpNE) {
|
||||
bitmask, ok := cond.Value.Value.(int64)
|
||||
if !ok {
|
||||
match = false
|
||||
break
|
||||
}
|
||||
for _, sub := range arr.SubORConditions {
|
||||
if rowMatchesCondition(row, sub) {
|
||||
fieldMatch = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if fieldMatch {
|
||||
if !MatchesBitmaskCondition(row, cond.Column, bitmask, cond.Operator) {
|
||||
match = false
|
||||
break
|
||||
}
|
||||
} else {
|
||||
if !MatchesCondition(row, cond) {
|
||||
match = false
|
||||
break
|
||||
}
|
||||
}
|
||||
if !fieldMatch {
|
||||
match = false
|
||||
break
|
||||
}
|
||||
}
|
||||
if match {
|
||||
|
|
|
|||
|
|
@ -6,177 +6,58 @@ import (
|
|||
"apiserver/utils"
|
||||
)
|
||||
|
||||
// makeJobRow builds a row in the OrderMap shape QueryData produces for a job.
|
||||
func makeJobRow(jobid int64, status int) utils.OrderMap {
|
||||
return utils.OrderMap{
|
||||
Fields: []string{"job_id", "job_status", "submit_user", "queue_name"},
|
||||
Data: map[string]interface{}{
|
||||
"job_id": jobid,
|
||||
"job_status": status,
|
||||
"submit_user": "admin",
|
||||
"queue_name": "normal",
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// bug-1504: `xcecli jobs list -a` (job_status notnull) filtered out EVERY job.
|
||||
// job_status is a bitmask field, so FilterRows routed the OpNOTNULL condition
|
||||
// to MatchesBitmaskCondition, which only implements OpEQ/OpNE bitmask
|
||||
// semantics and fell through to `return false` — the "all jobs" query always
|
||||
// returned "no job found".
|
||||
func TestFilterRows_JobStatusNotNull_MatchesAllJobs(t *testing.T) {
|
||||
// bug-1680: FilterRows unconditionally routes any condition on a bitmask
|
||||
// field (job_status/host_status) through MatchesBitmaskCondition, regardless
|
||||
// of the condition's actual operator. MatchesBitmaskCondition only knows how
|
||||
// to evaluate OpEQ/OpNE (the only shapes buildIntCondition's in/notin bitmask
|
||||
// branch ever produces) and silently returns false for anything else --
|
||||
// including OpNOTNULL, the shape xcecli's `-a` flag produces via
|
||||
// `{job_status:notnull:}`. The result: `-a` matches zero rows, every time,
|
||||
// regardless of how many jobs actually exist.
|
||||
func TestFilterRows_JobStatusNotNull_MatchesRealRows(t *testing.T) {
|
||||
rows := []utils.OrderMap{
|
||||
makeJobRow(1, 0x02), // RUN
|
||||
makeJobRow(2, 0x04), // EXIT
|
||||
makeJobRow(3, 0x08), // DONE
|
||||
{Fields: []string{"job_status"}, Data: map[string]interface{}{"job_status": int64(1)}}, // PEND
|
||||
{Fields: []string{"job_status"}, Data: map[string]interface{}{"job_status": int64(2)}}, // RUN
|
||||
{Fields: []string{"job_status"}, Data: map[string]interface{}{"job_status": int64(8)}}, // DONE
|
||||
}
|
||||
|
||||
// Mirrors exactly what buildIntCondition's null/notnull scalar path
|
||||
// produces for `{job_status:notnull:}` (types.go): Operator: OpNOTNULL,
|
||||
// Value: {TypeInt, 0} -- the 0 is a placeholder, never meant to be
|
||||
// interpreted as a bitmask.
|
||||
cond := utils.Condition{
|
||||
Column: "job_status",
|
||||
Operator: utils.OpNOTNULL,
|
||||
Value: utils.Value{Type: utils.TypeInt, Value: int64(0)},
|
||||
}
|
||||
|
||||
got := FilterRows(rows, []utils.Condition{cond})
|
||||
if len(got) != len(rows) {
|
||||
t.Fatalf("job_status notnull: got %d rows, want %d (all jobs have a status)", len(got), len(rows))
|
||||
result := FilterRows(rows, []utils.Condition{cond})
|
||||
|
||||
if len(result) != len(rows) {
|
||||
t.Errorf("FilterRows with job_status:notnull = %d rows, want %d (every row has a non-nil job_status)", len(result), len(rows))
|
||||
}
|
||||
}
|
||||
|
||||
func TestFilterRows_JobStatusIsNull_MatchesNothing(t *testing.T) {
|
||||
// Regression coverage: the actual bitmask shape (OpEQ/OpNE, produced by
|
||||
// buildIntCondition's in/notin branch) must keep working exactly as before.
|
||||
func TestFilterRows_JobStatusBitmaskEQ_StillWorks(t *testing.T) {
|
||||
rows := []utils.OrderMap{
|
||||
makeJobRow(1, 0x02),
|
||||
makeJobRow(2, 0x04),
|
||||
{Fields: []string{"job_status"}, Data: map[string]interface{}{"job_status": int64(1)}}, // PEND
|
||||
{Fields: []string{"job_status"}, Data: map[string]interface{}{"job_status": int64(2)}}, // RUN
|
||||
}
|
||||
|
||||
// Mirrors buildIntCondition's bitmask branch for `{job_status:in:(PEND)}`:
|
||||
// op = OpNE, Value = the OR'd bitmask of the requested status names.
|
||||
cond := utils.Condition{
|
||||
Column: "job_status",
|
||||
Operator: utils.OpISNULL,
|
||||
Value: utils.Value{Type: utils.TypeInt, Value: int64(0)},
|
||||
Operator: utils.OpNE,
|
||||
Value: utils.Value{Type: utils.TypeInt, Value: int64(1)},
|
||||
}
|
||||
|
||||
got := FilterRows(rows, []utils.Condition{cond})
|
||||
if len(got) != 0 {
|
||||
t.Fatalf("job_status null: got %d rows, want 0", len(got))
|
||||
}
|
||||
}
|
||||
|
||||
// Regression guard: the default list filter (job_status notin EXIT/DONE →
|
||||
// bitmask OpEQ with mask 0x0C) must keep working after the null/notnull fix.
|
||||
func TestFilterRows_JobStatusNotinExitDone(t *testing.T) {
|
||||
rows := []utils.OrderMap{
|
||||
makeJobRow(1, 0x02), // RUN — should survive
|
||||
makeJobRow(2, 0x04), // EXIT — filtered
|
||||
makeJobRow(3, 0x0C), // EXIT|DONE — filtered
|
||||
}
|
||||
|
||||
// notin (EXIT,DONE) on a bitmask field is compiled to OpEQ with mask
|
||||
// EXIT|DONE = 0x0C: keep rows where (val & mask) == 0.
|
||||
cond := utils.Condition{
|
||||
Column: "job_status",
|
||||
Operator: utils.OpEQ,
|
||||
Value: utils.Value{Type: utils.TypeInt, Value: int64(0x0C)},
|
||||
}
|
||||
|
||||
got := FilterRows(rows, []utils.Condition{cond})
|
||||
if len(got) != 1 || got[0].Data["job_status"] != 0x02 {
|
||||
t.Fatalf("job_status notin(EXIT,DONE): got %d rows, want exactly the RUN job", len(got))
|
||||
}
|
||||
}
|
||||
|
||||
// makeJobRowIdx builds a job row with a job_id and array_idx, as StoreToOrderMaps
|
||||
// produces for array-job rows.
|
||||
func makeJobRowIdx(jobid int64, arrayIdx int, status int) utils.OrderMap {
|
||||
row := makeJobRow(jobid, status)
|
||||
row.Data["array_idx"] = arrayIdx
|
||||
return row
|
||||
}
|
||||
|
||||
// bug-1490-adjacent: ExecuteQuery ignored CompConditions, so GET /xce/v1/jobs/:jobids
|
||||
// (GetJobByIDs) never applied its job_id filter and returned the ENTIRE job
|
||||
// table. The mcp scheduler_get_job tool therefore returned the first job in the
|
||||
// store regardless of the requested id.
|
||||
func TestExecuteQuery_CompConditions_JobIDFilter(t *testing.T) {
|
||||
rows := []utils.OrderMap{
|
||||
makeJobRow(1, 0x08), // DONE
|
||||
makeJobRow(2, 0x04), // EXIT
|
||||
makeJobRow(3, 0x02), // RUN
|
||||
}
|
||||
comp := utils.CompCondition{
|
||||
"job_id": []utils.ConditionArray{
|
||||
{
|
||||
MainCondition: utils.Condition{
|
||||
Column: "job_id",
|
||||
Operator: utils.OpEQ,
|
||||
Value: utils.Value{Type: utils.TypeInt, Value: int64(2)},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
got, _ := ExecuteQuery(rows, &utils.QueryOptions{CompConditions: comp}, nil)
|
||||
if len(got) != 1 || got[0].Data["job_id"] != int64(2) {
|
||||
t.Fatalf("comp job_id eq 2: got %d rows %v, want only job 2", len(got), got)
|
||||
}
|
||||
}
|
||||
|
||||
// Array-job id queries carry array_idx sub-ranges as OR conditions under the
|
||||
// job_id main condition (e.g. "100[1-3,5-7]"). A row matches when the main
|
||||
// condition holds AND any sub-condition holds.
|
||||
func TestExecuteQuery_CompConditions_ArrayIdxSubRanges(t *testing.T) {
|
||||
rows := []utils.OrderMap{}
|
||||
for _, idx := range []int{1, 2, 3, 4, 5, 6, 7} {
|
||||
rows = append(rows, makeJobRowIdx(100, idx, 0x01)) // PEND
|
||||
}
|
||||
comp := utils.CompCondition{
|
||||
"job_id": []utils.ConditionArray{
|
||||
{
|
||||
MainCondition: utils.Condition{
|
||||
Column: "job_id",
|
||||
Operator: utils.OpEQ,
|
||||
Value: utils.Value{Type: utils.TypeInt, Value: int64(100)},
|
||||
},
|
||||
SubORConditions: []utils.Condition{
|
||||
{Column: "array_idx", Operator: utils.OpBETWEEN, Value: utils.Value{Type: utils.TypeArray, Value: []utils.Value{{Value: int64(1)}, {Value: int64(3)}}}},
|
||||
{Column: "array_idx", Operator: utils.OpIN, Value: utils.Value{Type: utils.TypeArray, Value: []utils.Value{{Value: int64(5)}, {Value: int64(7)}}}},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
got, _ := ExecuteQuery(rows, &utils.QueryOptions{CompConditions: comp}, nil)
|
||||
if len(got) != 5 {
|
||||
t.Fatalf("array-idx sub-ranges: got %d rows, want 5 (idx 1,2,3,5,7)", len(got))
|
||||
}
|
||||
wantIdx := map[any]bool{1: true, 2: true, 3: true, 5: true, 7: true}
|
||||
for _, row := range got {
|
||||
if !wantIdx[row.Data["array_idx"]] {
|
||||
t.Errorf("unexpected array_idx %v selected", row.Data["array_idx"])
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Multiple FilterConditionArrays under one field are OR-ed (e.g. two plain job
|
||||
// ids in one request): any array matching selects the row.
|
||||
func TestExecuteQuery_CompConditions_ORAcrossArrays(t *testing.T) {
|
||||
rows := []utils.OrderMap{
|
||||
makeJobRow(1, 0x08),
|
||||
makeJobRow(2, 0x04),
|
||||
makeJobRow(3, 0x02),
|
||||
}
|
||||
comp := utils.CompCondition{
|
||||
"job_id": []utils.ConditionArray{
|
||||
{
|
||||
MainCondition: utils.Condition{Column: "job_id", Operator: utils.OpEQ, Value: utils.Value{Type: utils.TypeInt, Value: int64(1)}},
|
||||
},
|
||||
{
|
||||
MainCondition: utils.Condition{Column: "job_id", Operator: utils.OpEQ, Value: utils.Value{Type: utils.TypeInt, Value: int64(3)}},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
got, _ := ExecuteQuery(rows, &utils.QueryOptions{CompConditions: comp}, nil)
|
||||
if len(got) != 2 {
|
||||
t.Fatalf("OR across arrays: got %d rows, want 2 (jobs 1 and 3)", len(got))
|
||||
result := FilterRows(rows, []utils.Condition{cond})
|
||||
|
||||
if len(result) != 1 {
|
||||
t.Fatalf("FilterRows with job_status bitmask EQ = %d rows, want 1", len(result))
|
||||
}
|
||||
if result[0].Data["job_status"] != int64(1) {
|
||||
t.Errorf("matched row job_status = %v, want 1 (PEND)", result[0].Data["job_status"])
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue