fix: prevent cache pollution from empty-vector objects in HNSW lazy mode
Add comprehensive validation to detect and reject cached objects with empty vectors (vector: []) that can occur during HNSW adaptive caching with large datasets. Invalid cached objects are now automatically removed from cache and reloaded from storage. Changes: - GCS: Add empty vector validation (vector.length === 0 check) - GCS: Prevent caching nodes with empty vectors in saveNode() - GCS: Add detailed cached object structure logging - GCS: Auto-remove invalid cached objects - S3: Add same validation logic as GCS - Both: Fall through to storage loading when cache is invalid This fixes silent failures where getNode() returned null despite valid data existing in cloud storage. The root cause was empty-vector objects from HNSW's lazy-loading mode being cached and returned as valid, causing entity retrieval to fail after container restarts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
64fcaf3bf8
commit
91d233e1da
2 changed files with 74 additions and 17 deletions
|
|
@ -462,8 +462,13 @@ export class GcsStorage extends BaseStorage {
|
||||||
resumable: false // For small objects, non-resumable is faster
|
resumable: false // For small objects, non-resumable is faster
|
||||||
})
|
})
|
||||||
|
|
||||||
// Update cache
|
// CRITICAL FIX (v3.37.8): Only cache nodes with non-empty vectors
|
||||||
this.nounCacheManager.set(node.id, node)
|
// This prevents cache pollution from HNSW's lazy-loading nodes (vector: [])
|
||||||
|
if (node.vector && Array.isArray(node.vector) && node.vector.length > 0) {
|
||||||
|
this.nounCacheManager.set(node.id, node)
|
||||||
|
} else {
|
||||||
|
prodLog.warn(`[saveNode] Not caching node ${node.id.substring(0, 8)}... with empty vector (HNSW lazy mode)`)
|
||||||
|
}
|
||||||
|
|
||||||
// Increment noun count
|
// Increment noun count
|
||||||
const metadata = await this.getNounMetadata(node.id)
|
const metadata = await this.getNounMetadata(node.id)
|
||||||
|
|
@ -515,7 +520,7 @@ export class GcsStorage extends BaseStorage {
|
||||||
await this.ensureInitialized()
|
await this.ensureInitialized()
|
||||||
|
|
||||||
// Check cache first WITH LOGGING
|
// Check cache first WITH LOGGING
|
||||||
const cached = this.nounCacheManager.get(id)
|
const cached: HNSWNode | null = await this.nounCacheManager.get(id)
|
||||||
|
|
||||||
// DIAGNOSTIC LOGGING: Reveal cache poisoning
|
// DIAGNOSTIC LOGGING: Reveal cache poisoning
|
||||||
prodLog.info(`[getNode] 🔍 Cache check for ${id.substring(0, 8)}...:`, {
|
prodLog.info(`[getNode] 🔍 Cache check for ${id.substring(0, 8)}...:`, {
|
||||||
|
|
@ -525,11 +530,38 @@ export class GcsStorage extends BaseStorage {
|
||||||
type: typeof cached
|
type: typeof cached
|
||||||
})
|
})
|
||||||
|
|
||||||
// CRITICAL FIX: Only return cached value if it's valid (not null/undefined)
|
// CRITICAL FIX (v3.37.8): Validate cached object before returning
|
||||||
if (cached !== undefined && cached !== null) {
|
if (cached !== undefined && cached !== null) {
|
||||||
prodLog.info(`[getNode] ✅ Cache HIT - returning cached node for ${id.substring(0, 8)}...`)
|
// Log cached object structure to diagnose incomplete objects
|
||||||
this.logger.trace(`Cache hit for noun ${id}`)
|
prodLog.info(`[getNode] Cached object structure:`, {
|
||||||
return cached
|
hasId: !!cached.id,
|
||||||
|
idMatches: cached.id === id,
|
||||||
|
hasVector: !!cached.vector,
|
||||||
|
vectorLength: cached.vector?.length,
|
||||||
|
hasConnections: !!cached.connections,
|
||||||
|
connectionsType: typeof cached.connections,
|
||||||
|
hasLevel: cached.level !== undefined,
|
||||||
|
level: cached.level,
|
||||||
|
objectKeys: Object.keys(cached || {})
|
||||||
|
})
|
||||||
|
|
||||||
|
// Validate cached object has required fields (including non-empty vector!)
|
||||||
|
if (!cached.id || !cached.vector || !Array.isArray(cached.vector) || cached.vector.length === 0) {
|
||||||
|
prodLog.error(`[getNode] ❌ INVALID cached object for ${id.substring(0, 8)}...:`, {
|
||||||
|
reason: !cached.id ? 'missing id' :
|
||||||
|
!cached.vector ? 'missing vector' :
|
||||||
|
!Array.isArray(cached.vector) ? 'vector not array' :
|
||||||
|
cached.vector.length === 0 ? 'vector is empty array' :
|
||||||
|
'unknown'
|
||||||
|
})
|
||||||
|
prodLog.error(`[getNode] Removing invalid object from cache and loading from GCS`)
|
||||||
|
this.nounCacheManager.delete(id)
|
||||||
|
// Fall through to load from GCS
|
||||||
|
} else {
|
||||||
|
prodLog.info(`[getNode] ✅ Valid cached object - returning`)
|
||||||
|
this.logger.trace(`Cache hit for noun ${id}`)
|
||||||
|
return cached
|
||||||
|
}
|
||||||
} else if (cached === null) {
|
} else if (cached === null) {
|
||||||
prodLog.warn(`[getNode] ⚠️ Cache contains NULL for ${id.substring(0, 8)}... - ignoring and loading from GCS`)
|
prodLog.warn(`[getNode] ⚠️ Cache contains NULL for ${id.substring(0, 8)}... - ignoring and loading from GCS`)
|
||||||
} else {
|
} else {
|
||||||
|
|
@ -579,12 +611,12 @@ export class GcsStorage extends BaseStorage {
|
||||||
// NO metadata field - retrieved separately for scalability
|
// NO metadata field - retrieved separately for scalability
|
||||||
}
|
}
|
||||||
|
|
||||||
// CRITICAL FIX: Only cache valid nodes (never cache null)
|
// CRITICAL FIX: Only cache valid nodes with non-empty vectors (never cache null or empty)
|
||||||
if (node && node.id && node.vector && Array.isArray(node.vector)) {
|
if (node && node.id && node.vector && Array.isArray(node.vector) && node.vector.length > 0) {
|
||||||
this.nounCacheManager.set(id, node)
|
this.nounCacheManager.set(id, node)
|
||||||
prodLog.info(`[getNode] 💾 Cached node ${id.substring(0, 8)}... successfully`)
|
prodLog.info(`[getNode] 💾 Cached node ${id.substring(0, 8)}... successfully`)
|
||||||
} else {
|
} else {
|
||||||
prodLog.warn(`[getNode] ⚠️ NOT caching invalid node for ${id.substring(0, 8)}...`)
|
prodLog.warn(`[getNode] ⚠️ NOT caching invalid node for ${id.substring(0, 8)}... (missing id/vector or empty vector)`)
|
||||||
}
|
}
|
||||||
|
|
||||||
this.logger.trace(`Successfully retrieved node ${id}`)
|
this.logger.trace(`Successfully retrieved node ${id}`)
|
||||||
|
|
|
||||||
|
|
@ -1094,11 +1094,36 @@ export class S3CompatibleStorage extends BaseStorage {
|
||||||
type: typeof cached
|
type: typeof cached
|
||||||
})
|
})
|
||||||
|
|
||||||
// CRITICAL FIX: Only return cached value if it's valid (not null/undefined)
|
// CRITICAL FIX (v3.37.8): Validate cached object before returning
|
||||||
if (cached !== undefined && cached !== null) {
|
if (cached !== undefined && cached !== null) {
|
||||||
prodLog.info(`[getNode] ✅ Cache HIT - returning cached node for ${id.substring(0, 8)}...`)
|
// Log cached object structure to diagnose incomplete objects
|
||||||
this.logger.trace(`Cache hit for node ${id}`)
|
prodLog.info(`[getNode] Cached object structure:`, {
|
||||||
return cached
|
hasId: !!cached.id,
|
||||||
|
idMatches: cached.id === id,
|
||||||
|
hasVector: !!cached.vector,
|
||||||
|
vectorLength: cached.vector?.length,
|
||||||
|
hasConnections: !!cached.connections,
|
||||||
|
connectionsType: typeof cached.connections,
|
||||||
|
objectKeys: Object.keys(cached || {})
|
||||||
|
})
|
||||||
|
|
||||||
|
// Validate cached object has required fields (including non-empty vector!)
|
||||||
|
if (!cached.id || !cached.vector || !Array.isArray(cached.vector) || cached.vector.length === 0) {
|
||||||
|
prodLog.error(`[getNode] ❌ INVALID cached object for ${id.substring(0, 8)}...:`, {
|
||||||
|
reason: !cached.id ? 'missing id' :
|
||||||
|
!cached.vector ? 'missing vector' :
|
||||||
|
!Array.isArray(cached.vector) ? 'vector not array' :
|
||||||
|
cached.vector.length === 0 ? 'vector is empty array' :
|
||||||
|
'unknown'
|
||||||
|
})
|
||||||
|
prodLog.error(`[getNode] Removing invalid object from cache and loading from S3`)
|
||||||
|
this.nodeCache.delete(id)
|
||||||
|
// Fall through to load from S3
|
||||||
|
} else {
|
||||||
|
prodLog.info(`[getNode] ✅ Valid cached object - returning`)
|
||||||
|
this.logger.trace(`Cache hit for node ${id}`)
|
||||||
|
return cached
|
||||||
|
}
|
||||||
} else if (cached === null) {
|
} else if (cached === null) {
|
||||||
prodLog.warn(`[getNode] ⚠️ Cache contains NULL for ${id.substring(0, 8)}... - ignoring and loading from S3`)
|
prodLog.warn(`[getNode] ⚠️ Cache contains NULL for ${id.substring(0, 8)}... - ignoring and loading from S3`)
|
||||||
} else {
|
} else {
|
||||||
|
|
@ -1169,12 +1194,12 @@ export class S3CompatibleStorage extends BaseStorage {
|
||||||
level: parsedNode.level || 0
|
level: parsedNode.level || 0
|
||||||
}
|
}
|
||||||
|
|
||||||
// CRITICAL FIX: Only cache valid nodes (never cache null)
|
// CRITICAL FIX: Only cache valid nodes with non-empty vectors (never cache null or empty)
|
||||||
if (node && node.id && node.vector && Array.isArray(node.vector)) {
|
if (node && node.id && node.vector && Array.isArray(node.vector) && node.vector.length > 0) {
|
||||||
this.nodeCache.set(id, node)
|
this.nodeCache.set(id, node)
|
||||||
prodLog.info(`[getNode] 💾 Cached node ${id.substring(0, 8)}... successfully`)
|
prodLog.info(`[getNode] 💾 Cached node ${id.substring(0, 8)}... successfully`)
|
||||||
} else {
|
} else {
|
||||||
prodLog.warn(`[getNode] ⚠️ NOT caching invalid node for ${id.substring(0, 8)}...`)
|
prodLog.warn(`[getNode] ⚠️ NOT caching invalid node for ${id.substring(0, 8)}... (missing id/vector or empty vector)`)
|
||||||
}
|
}
|
||||||
|
|
||||||
this.logger.trace(`Successfully retrieved node ${id}`)
|
this.logger.trace(`Successfully retrieved node ${id}`)
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue