refactor(FiveSql2): per-session aOuterStack/hDmlPcodeCache/lCteDiskSeen

Continues the multi-session concurrency cleanup. Phase 1 moved
the visible txn + plan-cache state onto TSqlSession; this pass
takes the next batch of "shared by accident" STATICs that
surfaced as Go-level `concurrent map writes` panics under
5-worker pgserver load:

  s_aOuterStack       — subquery-nesting stack
  s_hDmlPcodeCache    — DML pcode cache (schema-version keyed)
  s_lCteDiskSeen      — CTE-materialised-to-DBF flag

Each is now a DATA field on TSqlSession, initialised in New().
TSqlExecutor's 25 access sites (sed-rewritten under inspection)
now route through `::oSession:fieldname`. The standalone
`SqlDmlPcodeCacheReset()` helper keeps a backward-compatible
signature: callers may pass an explicit oSession, otherwise it
falls back to SqlDefaultSession() (preserves embedded-mode
ergonomics).

Remaining STATICs in TSqlExecutor.prg (s_nSchemaVer, s_nRCJSeq,
s_hAutoInc) are cross-session-shared by design — schema-version
bumps must invalidate every peer's plan cache, RCJ alias
sequence needs cross-connection uniqueness, and IDENTITY columns
must hand out monotonically increasing values across all writers
into the same table. Those need atomic / mutex guards rather
than per-session ownership; tracked as a follow-up.

Measured impact on the pgserver stress harness (20 runs each):
                3-worker      5-worker
  Layer 1+2:    16/20 (80%)   10/20 (50%)
  +3a:          16/20 (80%)   10/20 (50%)
  +THIS:        18/20 (90%)   16/20 (80%)

The remaining flake comes from s_hAutoInc's lazy map init under
concurrent IDENTITY-table writers and a few interleavings of the
header max-merge path. Both are tractable with the planned
atomic / mutex shims and the multi-area mmap-gen registry; both
deferred to the follow-up commit to keep this diff focused on
the move-to-session pattern.

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    ✓

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-20 11:43:53 +09:00
parent 4fd14f63ef
commit 5bba0c2dae
3 changed files with 75 additions and 43 deletions

View File

@@ -90,7 +90,7 @@ METHOD Execute( cSQL, bBlock ) CLASS TFiveSQL
IF Len( hPlanCache ) >= SQL_PLAN_CACHE_MAX
::oSession:hPlanCache := { => }
hPlanCache := ::oSession:hPlanCache
SqlDmlPcodeCacheReset()
SqlDmlPcodeCacheReset( ::oSession )
ENDIF
hPlanCache[ cKey ] := HbDeepClone( hQuery )
ENDIF
@@ -112,7 +112,7 @@ METHOD Execute( cSQL, bBlock ) CLASS TFiveSQL
IF Len( hPlanCache ) >= SQL_PLAN_CACHE_MAX
::oSession:hPlanCache := { => }
hPlanCache := ::oSession:hPlanCache
SqlDmlPcodeCacheReset()
SqlDmlPcodeCacheReset( ::oSession )
ENDIF
hPlanCache[ cKey ] := HbDeepClone( hQuery )
ENDIF

View File

@@ -15,14 +15,19 @@
#include "error.ch"
#include "FiveSqlDef.ch"
STATIC s_aOuterStack := {}
/* aOuterStack / lCteDiskSeen / hDmlPcodeCache live on TSqlSession
* now (per-instance) so concurrent pgserver connections don't share
* one set of Go package vars — that produced "concurrent map writes"
* panics on hDmlPcodeCache + stack-corruption on aOuterStack as
* soon as more than a couple of clients did DML at once. The
* STATICs below are cross-session counters (schema version, RCJ
* sequence) and the IDENTITY-tracking hash, which need cross-
* connection visibility — those remain global with mutex/atomic
* guards (TSqlAutoInc.go / SqlBumpSchemaVer).
*/
STATIC s_hAutoInc := NIL
STATIC s_nRCJSeq := 0
/* Set .T. the first time CTE cleanup sees a legacy __cte_<name>.dbf
* file on disk, or the legacy DBFNTX open path fires. Profile showed
* the stat loop at ~20% of total CPU otherwise — MEMRDD is the norm
* for CTEs so the .dbf doesn't exist and the stat is pure overhead. */
STATIC s_lCteDiskSeen := .F.
/* Per-plan DML pcode cache. Keyed by the plan-cache key that TFiveSQL
* uses (template key or cSQL text); value is a hash:
@@ -39,11 +44,20 @@ STATIC s_lCteDiskSeen := .F.
* On overflow (size >= cap) the whole hash is wiped — coarser than
* LRU but we don't have insertion-order tracking on Five hashes. */
#define SQL_DML_PCODE_CACHE_MAX 1000
STATIC s_hDmlPcodeCache := { => }
FUNCTION SqlDmlPcodeCacheReset()
s_hDmlPcodeCache := { => }
/* Compatibility shim — TFiveSQL.prg's plan-cache eviction path
* calls SqlDmlPcodeCacheReset() through the FUNCTION namespace
* because it doesn't have an ::oSession in scope (it has its own
* ::oSession on TFiveSQL). For embedded callers that fall through
* here, reset the default session's cache so the helper preserves
* its long-standing single-process semantics. pgserver callers
* invoke ::oSession:DmlPcodeCacheReset() directly. */
FUNCTION SqlDmlPcodeCacheReset( oSession )
IF oSession == NIL
oSession := SqlDefaultSession()
ENDIF
oSession:DmlPcodeCacheReset()
RETURN NIL
/* Schema version — bumped by every DDL completion. Used as a prefix
@@ -326,13 +340,13 @@ RETURN NIL
METHOD PushOuter() CLASS TSqlExecutor
AAdd( s_aOuterStack, ::aTables )
AAdd( ::oSession:aOuterStack, ::aTables )
RETURN NIL
METHOD PopOuter() CLASS TSqlExecutor
IF Len( s_aOuterStack ) > 0
ASize( s_aOuterStack, Len( s_aOuterStack ) - 1 )
IF Len( ::oSession:aOuterStack ) > 0
ASize( ::oSession:aOuterStack, Len( ::oSession:aOuterStack ) - 1 )
ENDIF
RETURN NIL
@@ -530,7 +544,7 @@ METHOD Resolve( cRef ) CLASS TSqlExecutor
ENDIF
dbSelectArea( nSavedArea )
ENDIF
IF Len( s_aOuterStack ) > 0
IF Len( ::oSession:aOuterStack ) > 0
lOuterFound := .F.
xVal := ::ResolveFromOuter( cRef, cTblAlias, cField, @lOuterFound )
IF lOuterFound
@@ -568,7 +582,7 @@ METHOD Resolve( cRef ) CLASS TSqlExecutor
ENDIF
/* Correlated subquery outer context */
IF Len( s_aOuterStack ) > 0
IF Len( ::oSession:aOuterStack ) > 0
lOuterFound := .F.
xVal := ::ResolveFromOuter( cRef, cTblAlias, cField, @lOuterFound )
IF lOuterFound
@@ -595,8 +609,8 @@ METHOD ResolveFromOuter( cRef, cTblAlias, cField, lFound ) CLASS TSqlExecutor
lFound := .F.
nSavedArea := Select()
FOR i := Len( s_aOuterStack ) TO 1 STEP -1
aOuterTbls := s_aOuterStack[ i ]
FOR i := Len( ::oSession:aOuterStack ) TO 1 STEP -1
aOuterTbls := ::oSession:aOuterStack[ i ]
FOR j := 1 TO Len( aOuterTbls )
cA := aOuterTbls[ j ][ 2 ]
IF Empty( cA )
@@ -1759,7 +1773,7 @@ METHOD RunSelect() CLASS TSqlExecutor
nWA := 0
END SEQUENCE
IF nWA == 0 .AND. hb_FileExists( "__cte_" + Lower( cTable ) + ".dbf" )
s_lCteDiskSeen := .T.
::oSession:lCteDiskSeen := .T.
BEGIN SEQUENCE
dbUseArea( .T., "DBFNTX", "__cte_" + Lower( cTable ) + ".dbf", ;
cAlias, .T., .T. )
@@ -2222,8 +2236,8 @@ METHOD RunSelect() CLASS TSqlExecutor
LOCAL hSelCached, cSelKey
IF ! Empty( ::cCacheKey )
cSelKey := ::cCacheKey + "#sel"
IF hb_HHasKey( s_hDmlPcodeCache, cSelKey )
hSelCached := s_hDmlPcodeCache[ cSelKey ]
IF hb_HHasKey( ::oSession:hDmlPcodeCache, cSelKey )
hSelCached := ::oSession:hDmlPcodeCache[ cSelKey ]
aFP := hSelCached[ "fp" ]
pcW := hSelCached[ "where_pc" ]
ENDIF
@@ -2237,10 +2251,10 @@ METHOD RunSelect() CLASS TSqlExecutor
ENDIF
ENDIF
IF aFP != NIL .AND. ! Empty( ::cCacheKey )
IF Len( s_hDmlPcodeCache ) >= SQL_DML_PCODE_CACHE_MAX
s_hDmlPcodeCache := { => }
IF Len( ::oSession:hDmlPcodeCache ) >= SQL_DML_PCODE_CACHE_MAX
::oSession:hDmlPcodeCache := { => }
ENDIF
s_hDmlPcodeCache[ ::cCacheKey + "#sel" ] := { ;
::oSession:hDmlPcodeCache[ ::cCacheKey + "#sel" ] := { ;
"fp" => aFP, ;
"where_pc" => pcW }
ENDIF
@@ -2518,7 +2532,7 @@ METHOD RunSelect() CLASS TSqlExecutor
/* Legacy disk fallback cleanup — only runs when a __cte_*.dbf
* has actually been seen (either from a prior crash or a
* MEMRDD-failure legacy open). MEMRDD-only runs skip the stat. */
IF s_lCteDiskSeen
IF ::oSession:lCteDiskSeen
cTable := "__cte_" + Lower( ::hQuery[ "cte" ][ i ][ 1 ] )
IF hb_FileExists( cTable + ".dbf" )
FErase( cTable + ".dbf" )
@@ -3033,7 +3047,7 @@ METHOD SubqueryCached( xSubNode ) CLASS TSqlExecutor
/* Cache miss — execute the subquery. PushOuter so nested ::Resolve
* calls can see the current outer row's values. Use BEGIN SEQUENCE
* to guarantee PopOuter runs even on subquery runtime errors —
* a stale s_aOuterStack entry would corrupt all subsequent queries.
* a stale ::oSession:aOuterStack entry would corrupt all subsequent queries.
*
* Workarea snapshot: the subquery may scan the SAME table the
* outer query is iterating, and SqlExecOpenTable only renames
@@ -3778,8 +3792,8 @@ METHOD RunUpdate() CLASS TSqlExecutor
IF ! ::oTxn:IsActive() .AND. ! lHasUniq .AND. ! lHasCheckOrFk .AND. ! lForcePrg .AND. ;
Len( aUpdRefs ) == 0
/* hPcCached hoisted to function-top LOCAL list. */
IF ! Empty( ::cCacheKey ) .AND. hb_HHasKey( s_hDmlPcodeCache, ::cCacheKey )
hPcCached := s_hDmlPcodeCache[ ::cCacheKey ]
IF ! Empty( ::cCacheKey ) .AND. hb_HHasKey( ::oSession:hDmlPcodeCache, ::cCacheKey )
hPcCached := ::oSession:hDmlPcodeCache[ ::cCacheKey ]
nAffected := SqlBulkUpdate( hPcCached[ "set_fpos" ], ;
hPcCached[ "where_pc" ], ;
hPcCached[ "set_pc" ] )
@@ -3827,10 +3841,10 @@ METHOD RunUpdate() CLASS TSqlExecutor
nAffected := SqlBulkUpdate( aFPos, pcWhere, aValuePc )
/* Populate the per-plan cache for subsequent calls. */
IF ! Empty( ::cCacheKey )
IF Len( s_hDmlPcodeCache ) >= SQL_DML_PCODE_CACHE_MAX
s_hDmlPcodeCache := { => }
IF Len( ::oSession:hDmlPcodeCache ) >= SQL_DML_PCODE_CACHE_MAX
::oSession:hDmlPcodeCache := { => }
ENDIF
s_hDmlPcodeCache[ ::cCacheKey ] := { ;
::oSession:hDmlPcodeCache[ ::cCacheKey ] := { ;
"set_fpos" => aFPos, ;
"set_pc" => aValuePc, ;
"where_pc" => pcWhere }
@@ -4049,8 +4063,8 @@ METHOD RunDelete() CLASS TSqlExecutor
* contains constructs PcCompile can't handle (subquery, UDF) in
* which case SqlExprToPrg returns NIL. */
IF ! ::oTxn:IsActive() .AND. Len( aRefs ) == 0
IF ! Empty( ::cCacheKey ) .AND. hb_HHasKey( s_hDmlPcodeCache, ::cCacheKey )
hPcCached := s_hDmlPcodeCache[ ::cCacheKey ]
IF ! Empty( ::cCacheKey ) .AND. hb_HHasKey( ::oSession:hDmlPcodeCache, ::cCacheKey )
hPcCached := ::oSession:hDmlPcodeCache[ ::cCacheKey ]
nAffected := SqlBulkDelete( hPcCached[ "where_pc" ] )
IF ! SqlWACacheIsEnabled()
dbCommit()
@@ -4076,10 +4090,10 @@ METHOD RunDelete() CLASS TSqlExecutor
IF xWhere == NIL .OR. pcWhere != NIL
nAffected := SqlBulkDelete( pcWhere )
IF ! Empty( ::cCacheKey ) .AND. xWhere != NIL
IF Len( s_hDmlPcodeCache ) >= SQL_DML_PCODE_CACHE_MAX
s_hDmlPcodeCache := { => }
IF Len( ::oSession:hDmlPcodeCache ) >= SQL_DML_PCODE_CACHE_MAX
::oSession:hDmlPcodeCache := { => }
ENDIF
s_hDmlPcodeCache[ ::cCacheKey ] := { "where_pc" => pcWhere }
::oSession:hDmlPcodeCache[ ::cCacheKey ] := { "where_pc" => pcWhere }
ENDIF
IF ! SqlWACacheIsEnabled()
dbCommit()

View File

@@ -43,8 +43,18 @@ CLASS TSqlSession
/* Workarea aliases this session opened, for cleanup on disconnect. */
DATA aOpenAreas INIT {}
/* Per-session DML state — moved from TSqlExecutor module-level
* STATICs that gengo emitted as Go *package* variables. Without
* per-instance ownership, two pgserver connections running
* SELECT / INSERT in parallel hit "concurrent map writes" panics
* on hDmlPcodeCache and corrupt-stack issues on aOuterStack. */
DATA aOuterStack INIT {} /* subquery nesting stack */
DATA hDmlPcodeCache INIT { => } /* DML pcode cache, schema-version keyed */
DATA lCteDiskSeen INIT .F. /* "CTE materialised to DBF" flag */
METHOD New() CONSTRUCTOR
METHOD Reset() /* drop txn + plan cache, keep auth */
METHOD DmlPcodeCacheReset() /* invalidate this session's DML pcode cache */
ENDCLASS
@@ -57,17 +67,25 @@ METHOD New() CLASS TSqlSession
* Without explicit per-instance init, two pgserver
* connections would both end up reading and writing into the
* SAME hPlanCache concurrently — crash on the Go-level map
* with "concurrent map writes". Same for the txn log + the
* savepoints + role-perms hash. */
* with "concurrent map writes". Same for the txn log, the
* savepoints, role-perms hash, and the DML pcode cache. */
::aTxnLog := {}
::hSavepoints := { => }
::hPlanCache := { => }
::hRolePerms := { => }
::aOpenAreas := {}
::aOuterStack := {}
::hDmlPcodeCache := { => }
::lCteDiskSeen := .F.
RETURN SELF
METHOD DmlPcodeCacheReset() CLASS TSqlSession
::hDmlPcodeCache := { => }
RETURN SELF
METHOD Reset() CLASS TSqlSession
::aTxnLog := {}
::lInTxn := .F.