aokolnychyi commented on code in PR #53143:
URL: https://github.com/apache/spark/pull/53143#discussion_r2547504430


##########
sql/core/src/main/scala/org/apache/spark/sql/execution/CacheManager.scala:
##########
@@ -352,22 +353,35 @@ class CacheManager extends Logging with 
AdaptiveSparkPlanHelper {
     }
     needToRecache.foreach { cd =>
       cd.cachedRepresentation.cacheBuilder.clearCache()
+      tryRebuildCacheEntry(spark, cd).foreach { entry =>
+        this.synchronized {
+          if (lookupCachedDataInternal(entry.plan).nonEmpty) {
+            logWarning("While recaching, data was already added to cache.")
+          } else {
+            cachedData = entry +: cachedData
+            CacheManager.logCacheOperation(log"Re-cached Dataframe cache 
entry:" +
+              log"${MDC(DATAFRAME_CACHE_ENTRY, entry)}")
+          }
+        }
+      }
+    }
+  }
+
+  private def tryRebuildCacheEntry(
+      spark: SparkSession,
+      cd: CachedData): Option[CachedData] = {
+    try {
       val sessionWithConfigsOff = getOrCloneSessionWithConfigsOff(spark)
       val (newKey, newCache) = sessionWithConfigsOff.withActive {
         val refreshedPlan = V2TableRefreshUtil.refresh(cd.plan)

Review Comment:
   In 4.1, we added this line to refresh versions. This refresh MUST NOT fail 
writes.
   
   ```
   val refreshedPlan = V2TableRefreshUtil.refresh(cd.plan)
   ```
   
   I am not sure how we want to treat failures from the line below. Previously, 
this threw an exception and potentially marked writes as failed if we couldn't 
refresh.
   
   ```
   val qe = sessionWithConfigsOff.sessionState.executePlan(refreshedPlan)
   ```
   
   The current implementation will NOT throw an exception in either of the 
cases. Other options:
   - Don't fail only if refresh fails. Continue to fail if QE construction 
fails.
   - Don't fail in either case but only for DSv2 operations.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to