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:
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user