From 5bba0c2daecd4bd5b65d5a4a45c4bd20ec5e1398 Mon Sep 17 00:00:00 2001 From: CharlesKWON Date: Wed, 20 May 2026 11:43:53 +0900 Subject: [PATCH] refactor(FiveSql2): per-session aOuterStack/hDmlPcodeCache/lCteDiskSeen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- _FiveSql2/src/TFiveSQL.prg | 4 +- _FiveSql2/src/TSqlExecutor.prg | 82 ++++++++++++++++++++-------------- _FiveSql2/src/TSqlSession.prg | 32 ++++++++++--- 3 files changed, 75 insertions(+), 43 deletions(-) diff --git a/_FiveSql2/src/TFiveSQL.prg b/_FiveSql2/src/TFiveSQL.prg index 84e9ea1..9a34f3c 100644 --- a/_FiveSql2/src/TFiveSQL.prg +++ b/_FiveSql2/src/TFiveSQL.prg @@ -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 diff --git a/_FiveSql2/src/TSqlExecutor.prg b/_FiveSql2/src/TSqlExecutor.prg index ea3694a..0d87fe4 100644 --- a/_FiveSql2/src/TSqlExecutor.prg +++ b/_FiveSql2/src/TSqlExecutor.prg @@ -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_.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() diff --git a/_FiveSql2/src/TSqlSession.prg b/_FiveSql2/src/TSqlSession.prg index 2df354c..cd94c9a 100644 --- a/_FiveSql2/src/TSqlSession.prg +++ b/_FiveSql2/src/TSqlSession.prg @@ -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. */ - ::aTxnLog := {} - ::hSavepoints := { => } - ::hPlanCache := { => } - ::hRolePerms := { => } - ::aOpenAreas := {} + * 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.