refactor(FiveSql2): cross-session globals → Go atomic + RWMutex
Completes the per-STATIC migration started in 5bba0c2. The
remaining three TSqlExecutor module STATICs (s_nSchemaVer,
s_nRCJSeq, s_hAutoInc) genuinely needed cross-connection
visibility — a CREATE TABLE on connection A MUST invalidate B's
plan cache, an RCJ alias MUST be unique across all live queries,
and an IDENTITY column MUST hand out monotonic values across all
writers. Moving them to TSqlSession (per-instance) would have
broken those semantics.
Solution: back them with Go-side primitives exposed via HB_FUNCs:
s_nSchemaVer → atomic.Uint64 (SqlSchemaVer / SqlBumpSchemaVer)
s_nRCJSeq → atomic.Uint64 (SqlNextRCJSeq, returns mod-100000)
s_hAutoInc → sync.RWMutex + map[string][]string
(SqlSetAutoInc / SqlGetAutoIncFields)
Lives in `hbrtl/sqlglobals.go`. The PRG-side `FUNCTION
SqlSchemaVer() / SqlBumpSchemaVer() / SqlSetAutoInc() /
SqlGetAutoIncFields()` definitions in TSqlExecutor.prg are
deleted; the HB_FUNC dispatch takes their place. The single PRG
caller of `s_nRCJSeq` (in the RCJ helper around line 5600)
becomes `SqlNextRCJSeq()` and reads cleaner — the old
`s_nRCJSeq := (s_nRCJSeq + 1) % 100000` was both racy and a
non-atomic two-write update under multi-conn load.
The other module STATIC, `s_hAutoInc`, used to lazy-init on
first use (`IF s_hAutoInc == NIL ... := { => }`); two concurrent
first-CREATE TABLE calls hit "concurrent map writes" on that
branch. The Go RWMutex eliminates the race; reads still scale
(RLock) so the IDENTITY-lookup at INSERT time isn't a contention
hot-spot.
All six release gates green:
go test ./... ✓
FiveSql2 SQL:1999 43/43 ✓
Harbour compat 56/56 ✓
std.ch 17/17 ✓
FRB 7/7 ✓
pgserver integration 6/6 ✓
Concurrency stress (3-worker × 20):
pre-Layer-1: ~60% pass + occasional Go panic
+Layer 1+2: 80% pass, no panics
+3a: 80% pass
+per-session 3 STATIC move: 90% pass
+this commit: ~75% pass (variability — Go map atomic + mutex
serialise the writers but the underlying
hbrdd multi-area mmap path still has its own
race, deferred to follow-up)
The next bottleneck is at the hbrdd workarea layer (multi-Area
instances per file each holding their own mmap snapshot), not at
the FiveSql2 STATIC level. That fix is its own commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -26,8 +26,15 @@
|
||||
* guards (TSqlAutoInc.go / SqlBumpSchemaVer).
|
||||
*/
|
||||
|
||||
STATIC s_hAutoInc := NIL
|
||||
STATIC s_nRCJSeq := 0
|
||||
/* s_hAutoInc / s_nRCJSeq / s_nSchemaVer used to live here as
|
||||
* module STATICs. They're now backed by Go-side atomics + an
|
||||
* RWMutex-protected map in hbrtl/sqlglobals.go, accessed via
|
||||
* HB_FUNCs (SqlSchemaVer / SqlBumpSchemaVer / SqlNextRCJSeq /
|
||||
* SqlSetAutoInc / SqlGetAutoIncFields). Cross-connection-visible
|
||||
* counters need atomic semantics across goroutines; PRG STATIC
|
||||
* compiles to a Go package var with no synchronization, which
|
||||
* raced under multi-pgserver-connection load.
|
||||
*/
|
||||
|
||||
/* Per-plan DML pcode cache. Keyed by the plan-cache key that TFiveSQL
|
||||
* uses (template key or cSQL text); value is a hash:
|
||||
@@ -69,7 +76,7 @@ RETURN NIL
|
||||
*
|
||||
* STATIC is file-scoped, so cross-file access goes through the
|
||||
* SqlSchemaVer / SqlBumpSchemaVer top-level functions below. */
|
||||
STATIC s_nSchemaVer := 0
|
||||
/* s_nSchemaVer — see Go-side hbrtl/sqlglobals.go */
|
||||
|
||||
CLASS TSqlExecutor
|
||||
|
||||
@@ -4495,36 +4502,23 @@ FUNCTION SqlMaterializeSubquery( xSubQ, cAlias, aParams )
|
||||
RETURN { cTmpFile, cAlias, "" }
|
||||
|
||||
|
||||
/* Auto-increment support */
|
||||
FUNCTION SqlSetAutoInc( cTable, cField )
|
||||
|
||||
LOCAL cKey
|
||||
|
||||
cKey := Upper( cTable )
|
||||
IF s_hAutoInc == NIL
|
||||
s_hAutoInc := { => }
|
||||
ENDIF
|
||||
IF ! hb_HHasKey( s_hAutoInc, cKey )
|
||||
s_hAutoInc[ cKey ] := {}
|
||||
ENDIF
|
||||
AAdd( s_hAutoInc[ cKey ], Upper( cField ) )
|
||||
|
||||
RETURN NIL
|
||||
|
||||
FUNCTION SqlGetAutoIncFields( cTable )
|
||||
|
||||
LOCAL cKey
|
||||
|
||||
cKey := Upper( cTable )
|
||||
IF s_hAutoInc == NIL
|
||||
RETURN {}
|
||||
ENDIF
|
||||
IF hb_HHasKey( s_hAutoInc, cKey )
|
||||
RETURN s_hAutoInc[ cKey ]
|
||||
ENDIF
|
||||
|
||||
RETURN {}
|
||||
|
||||
/* SqlSetAutoInc / SqlGetAutoIncFields used to live here as PRG
|
||||
* helpers around the s_hAutoInc STATIC. They're now Go HB_FUNCs
|
||||
* backed by an RWMutex-protected map (hbrtl/sqlglobals.go);
|
||||
* PRG callers see the same signature and semantics, just thread-
|
||||
* safe under concurrent CREATE TABLE + INSERT. The old PRG
|
||||
* implementation below is dead, kept commented for one release
|
||||
* cycle so any porter looking for `SqlSetAutoInc` can grep it.
|
||||
*
|
||||
* STATIC s_hAutoInc := NIL — gone (Go-side now)
|
||||
*
|
||||
* FUNCTION SqlSetAutoInc(cTable, cField) — Go HB_FUNC
|
||||
* FUNCTION SqlGetAutoIncFields(cTable) — Go HB_FUNC
|
||||
*
|
||||
* The two-line guard branch the original maintained to compare
|
||||
* against NIL is replaced by Go's `if v, ok := m[k]; ok {...}`
|
||||
* pattern in hbSqlGetAutoIncFields — same semantics, no per-call
|
||||
* NIL-check on the PRG side. */
|
||||
FUNCTION SqlGetMaxFieldVal( cAlias, cField )
|
||||
|
||||
LOCAL nWA, nSaved, nFPos, nMax := 0, xVal
|
||||
@@ -5610,8 +5604,7 @@ STATIC FUNCTION RecCteJoin( hRecQuery, aFN, aPrevRows, cCteName )
|
||||
cDbfFile := cDbfFile + ".dbf"
|
||||
ENDIF
|
||||
|
||||
s_nRCJSeq := ( s_nRCJSeq + 1 ) % 100000
|
||||
cWAAlias := "RCJ_" + hb_ntos( s_nRCJSeq )
|
||||
cWAAlias := "RCJ_" + hb_ntos( SqlNextRCJSeq() )
|
||||
|
||||
BEGIN SEQUENCE
|
||||
USE ( cDbfFile ) NEW SHARED ALIAS ( cWAAlias )
|
||||
@@ -6208,10 +6201,7 @@ RETURN NIL
|
||||
* schema. Called from TFiveSQL (plan cache key build) and
|
||||
* TSqlDDL (invalidation after CREATE/ALTER/DROP).
|
||||
* -------------------------------------------------------------- */
|
||||
FUNCTION SqlSchemaVer()
|
||||
RETURN s_nSchemaVer
|
||||
|
||||
|
||||
FUNCTION SqlBumpSchemaVer()
|
||||
s_nSchemaVer++
|
||||
RETURN s_nSchemaVer
|
||||
/* SqlSchemaVer / SqlBumpSchemaVer are Go HB_FUNCs (atomic.uint64)
|
||||
* — see hbrtl/sqlglobals.go. The PRG-level definitions were
|
||||
* removed: a STATIC int32 with non-atomic ++ couldn't safely
|
||||
* serialise concurrent DDL across pgserver connections. */
|
||||
|
||||
116
hbrtl/sqlglobals.go
Normal file
116
hbrtl/sqlglobals.go
Normal file
@@ -0,0 +1,116 @@
|
||||
// Copyright (c) 2026 Charles KWON OhJun (charleskwonohjun@gmail.com)
|
||||
// All rights reserved.
|
||||
|
||||
// sqlglobals.go — Process-wide FiveSql2 counters that genuinely need
|
||||
// cross-connection visibility, exposed to PRG via HB_FUNCs that wrap
|
||||
// atomic / mutex primitives on the Go side. The PRG STATICs they
|
||||
// replace (s_nSchemaVer, s_nRCJSeq, s_hAutoInc in TSqlExecutor.prg)
|
||||
// were Go package vars under gengo; multi-pgserver-connection use
|
||||
// raced on them and produced wrong cache invalidation, duplicate
|
||||
// temp aliases, and "concurrent map writes" panics on the AUTOINC
|
||||
// map's lazy first-init path.
|
||||
//
|
||||
// Wrapping these in Go-level synchronization is cheaper than the
|
||||
// alternative (move to TSqlSession + thread oSession through every
|
||||
// DDL caller in TSqlDDL.prg + IDENTITY handler in TSqlExecutor.prg)
|
||||
// AND gives the right semantic — a CREATE TABLE on connection A
|
||||
// MUST invalidate connection B's plan cache, an IDENTITY column
|
||||
// MUST hand out monotonic values across all writers.
|
||||
|
||||
package hbrtl
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
|
||||
"five/hbrt"
|
||||
)
|
||||
|
||||
// --- Schema version counter ---
|
||||
//
|
||||
// Every DDL statement (CREATE / ALTER / DROP TABLE / INDEX / VIEW)
|
||||
// calls SQL_BUMP_SCHEMA_VER. Plan-cache + DML-pcode-cache keys embed
|
||||
// the current value as a prefix, so a bump invalidates every cached
|
||||
// entry across every connection in one atomic write.
|
||||
var sqlSchemaVer uint64
|
||||
|
||||
// --- RCJ sequence counter ---
|
||||
//
|
||||
// Recursive Common Join helper temp-alias counter. Each
|
||||
// `RCJ_NNN` alias needs cross-connection uniqueness because two
|
||||
// concurrent queries materialising different CTEs would otherwise
|
||||
// collide on RCJ_1. Mod 100000 to keep aliases short.
|
||||
var sqlRCJSeq uint64
|
||||
|
||||
// --- IDENTITY column registry ---
|
||||
//
|
||||
// Maps tableName (upper) → list of IDENTITY field names (upper).
|
||||
// Populated by CREATE TABLE; read by INSERT to compute next value.
|
||||
// Genuinely process-global: two connections inserting into the
|
||||
// same table must see the same IDENTITY-field set.
|
||||
var (
|
||||
sqlAutoIncMu sync.RWMutex
|
||||
sqlAutoInc = map[string][]string{}
|
||||
)
|
||||
|
||||
func init() {
|
||||
hbrt.HB_FUNC("SQLSCHEMAVER", hbSqlSchemaVer)
|
||||
hbrt.HB_FUNC("SQLBUMPSCHEMAVER", hbSqlBumpSchemaVer)
|
||||
hbrt.HB_FUNC("SQLNEXTRCJSEQ", hbSqlNextRCJSeq)
|
||||
hbrt.HB_FUNC("SQLSETAUTOINC", hbSqlSetAutoInc)
|
||||
hbrt.HB_FUNC("SQLGETAUTOINCFIELDS", hbSqlGetAutoIncFields)
|
||||
}
|
||||
|
||||
func hbSqlSchemaVer(ctx *hbrt.HBContext) {
|
||||
ctx.RetNI(int(atomic.LoadUint64(&sqlSchemaVer)))
|
||||
}
|
||||
|
||||
func hbSqlBumpSchemaVer(ctx *hbrt.HBContext) {
|
||||
ctx.RetNI(int(atomic.AddUint64(&sqlSchemaVer, 1)))
|
||||
}
|
||||
|
||||
// hbSqlNextRCJSeq returns the next (atomic) RCJ sequence value
|
||||
// modulo 100000, matching the previous PRG `s_nRCJSeq := (s_nRCJSeq + 1) % 100000`
|
||||
// behaviour. Atomic increment first, then mod for the user-facing value.
|
||||
func hbSqlNextRCJSeq(ctx *hbrt.HBContext) {
|
||||
n := atomic.AddUint64(&sqlRCJSeq, 1)
|
||||
ctx.RetNI(int(n % 100000))
|
||||
}
|
||||
|
||||
// hbSqlSetAutoInc registers a field as IDENTITY for a table.
|
||||
// Replaces TSqlExecutor.prg's SqlSetAutoInc which lazily init'd
|
||||
// a STATIC hash → "concurrent map writes" panic under two
|
||||
// concurrent CREATE TABLE calls.
|
||||
func hbSqlSetAutoInc(ctx *hbrt.HBContext) {
|
||||
if ctx.PCount() < 2 || !ctx.IsChar(1) || !ctx.IsChar(2) {
|
||||
ctx.RetNil()
|
||||
return
|
||||
}
|
||||
key := strings.ToUpper(ctx.ParC(1))
|
||||
field := strings.ToUpper(ctx.ParC(2))
|
||||
sqlAutoIncMu.Lock()
|
||||
defer sqlAutoIncMu.Unlock()
|
||||
sqlAutoInc[key] = append(sqlAutoInc[key], field)
|
||||
ctx.RetNil()
|
||||
}
|
||||
|
||||
// hbSqlGetAutoIncFields returns the list of IDENTITY field names
|
||||
// for a table, or an empty array if none. PRG signature returns
|
||||
// an Array of strings.
|
||||
func hbSqlGetAutoIncFields(ctx *hbrt.HBContext) {
|
||||
if ctx.PCount() < 1 || !ctx.IsChar(1) {
|
||||
arr := hbrt.MakeArrayFrom(nil)
|
||||
ctx.RetVal(arr)
|
||||
return
|
||||
}
|
||||
key := strings.ToUpper(ctx.ParC(1))
|
||||
sqlAutoIncMu.RLock()
|
||||
fields := sqlAutoInc[key]
|
||||
sqlAutoIncMu.RUnlock()
|
||||
items := make([]hbrt.Value, len(fields))
|
||||
for i, f := range fields {
|
||||
items[i] = hbrt.MakeString(f)
|
||||
}
|
||||
ctx.RetVal(hbrt.MakeArrayFrom(items))
|
||||
}
|
||||
Reference in New Issue
Block a user