Skip to content

[SPARK-44064][SQL] Move NonFateSharingCache from core module to catalyst module #41622

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
LuciferYang wants to merge 3 commits into from
Closed

[SPARK-44064][SQL] Move NonFateSharingCache from core module to catalyst module #41622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
968cd09
move NonFateSharingCache from core to catalyst
LuciferYang Jun 16, 2023
9b1dcb7
revert import change
LuciferYang Jun 16, 2023
1e75d67
Merge branch 'apache:master' into SPARK-44064
LuciferYang Jun 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ...lyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ import org.apache.spark.sql.catalyst.util.DateTimeConstants.NANOS_PER_MILLIS
import org.apache.spark.sql.errors.QueryExecutionErrors
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.types._
import org.apache.spark.sql.util.NonFateSharingCache
import org.apache.spark.unsafe.Platform
import org.apache.spark.unsafe.types._
import org.apache.spark.util.{LongAccumulator, NonFateSharingCache, ParentClassLoader, Utils}
import org.apache.spark.util.{LongAccumulator, ParentClassLoader, Utils}

/**
* Java source for evaluating an [[Expression]] given a [[InternalRow]] of input.
Expand Down
4 changes: 3 additions & 1 deletion ...ache/spark/util/NonFateSharingCache.scala → .../spark/sql/util/NonFateSharingCache.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@
* limitations under the License.
*/

package org.apache.spark.util
package org.apache.spark.sql.util
Copy link
Member

@dongjoon-hyun dongjoon-hyun Jun 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the code content, NonFateSharingCache fits org.apache.spark.util instead of org.apache.spark.sql.util because it's more general. WDYT?

Copy link
Member

@HyukjinKwon HyukjinKwon Jun 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree but seems like it causes some problem @LuciferYang pointed at here #40982 (comment) if we move this to other modules (and catalyst module only has sql module). It doesn't look so beautiful but I think this is a minimized fix.

Copy link
Member

@dongjoon-hyun dongjoon-hyun Jun 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, got it. I missed the context. Thanks!


import java.util.concurrent.Callable

import com.google.common.cache.Cache
import com.google.common.cache.LoadingCache

import org.apache.spark.util.KeyLock

/**
* SPARK-43300: Guava cache fate-sharing behavior might lead to unexpected cascade failure:
* when multiple threads access the same key in the cache at the same time when the key is not in
Expand Down
3 changes: 2 additions & 1 deletion ...spark/util/NonFateSharingCacheSuite.scala → ...k/sql/util/NonFateSharingCacheSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* limitations under the License.
*/

package org.apache.spark.util;
package org.apache.spark.sql.util;

import java.util.concurrent.ExecutionException
import java.util.concurrent.Semaphore
Expand All @@ -25,6 +25,7 @@ import com.google.common.cache.CacheBuilder
import com.google.common.cache.CacheLoader

import org.apache.spark.SparkFunSuite
import org.apache.spark.util.ThreadUtils

object NonFateSharingCacheSuite {
private val TEST_KEY = "key"
Expand Down