Full support for multithreaded applications #641

AFFogarty · 2020-08-25T18:12:53Z

Design

This PR enables multi-threaded user applications. The PR accomplishes this by creating a 1-to-1 mapping between .NET Thread objects and JVM single-thread ExecutorService objects. ExecutorService is a Java interface that wraps a worker thread with its own task queue.

When a .NET thread calls a Spark function, the JvmBridge includes the thread's ManagedThreadId in the payload body. When the request is received on the JVM side, DotnetBackendHandler uses the ManagedThreadId to map the request onto its corresponding ExecutorService. The ExecutorService then executes the function on its worker thread.

This pattern guarantees that Spark functions called by a particular .NET thread will always be executed on the same JVM thread. In addition, two different .NET threads will always execute Spark functions on two different JVM threads.

An object called ThreadPool manages the lifecycle for ExecutorService objects on the JVM side. A corresponding class called JvmThreadPool manages the threads on the .NET side, and does periodic garbage collection.

Additional features

This PR also enables GetActiveSession, SetActiveSession, and ClearActiveSession APIs on SparkSession. These APIs were previously unavailable because thread-local sessions were unsupported.

Testing

This PR adds a new E2E test class JvmThreadPoolTests that tests the basic functionality of the thread pool.

Since these changes are on the code path for all Spark API calls, these changes are exercised by all existing E2E tests.

Limitations

The only limitation is that in Spark, ActiveSession gives a guarantee that a child thread will inherit its parent's active session. My design does not support that. However, I think that this is not a problem since .NET's thread model doesn't support parent/child relationships between threads.

This PR will address bug #333

…hread_local

src/csharp/Microsoft.Spark/Interop/Ipc/JvmThreadPool.cs

src/csharp/Microsoft.Spark/Sql/SparkSession.cs

.../microsoft-spark-2.4.x/src/main/scala/org/apache/spark/api/dotnet/DotnetBackendHandler.scala

.../microsoft-spark-2.3.x/src/main/scala/org/apache/spark/api/dotnet/DotnetBackendHandler.scala

.../microsoft-spark-3.0.x/src/main/scala/org/apache/spark/api/dotnet/DotnetBackendHandler.scala

.../microsoft-spark-2.3.x/src/main/scala/org/apache/spark/api/dotnet/DotnetBackendHandler.scala

src/scala/microsoft-spark-2.3.x/src/main/scala/org/apache/spark/api/dotnet/ThreadPool.scala

…k/api/dotnet/ThreadPool.scala Co-authored-by: Steve Suh <[email protected]>

…hread_local

… into anfog/thread_local

Niharikadutta · 2020-10-03T01:18:03Z

src/csharp/Microsoft.Spark/Sql/SparkSession.cs

+        /// (first created) context.
+        /// </summary>
+        /// <param name="session">SparkSession object</param>
+        public static void SetActiveSession(SparkSession session) =>


Can we add the version attribute [Since(Versions.V3_0_0)] for this API?

Nevermind this comment, got confused by #633 and Pyspark but I see this has been exposed by Spark since 2.0.0 so please ignore this comment.

Niharikadutta · 2020-10-03T01:43:06Z

LGTM. Thanks @AFFogarty !

src/csharp/Microsoft.Spark.E2ETest/IpcTests/JvmThreadPoolGarbageCollectorTests.cs

src/csharp/Microsoft.Spark/Interop/Ipc/JvmBridge.cs

src/csharp/Microsoft.Spark/Services/IConfigurationService.cs

src/csharp/Microsoft.Spark/Interop/Ipc/JvmThreadPoolGarbageCollector.cs

src/csharp/Microsoft.Spark/Services/ConfigurationService.cs

…hread_local

imback82

Few nit/minor comments, but looking good to me!

src/csharp/Microsoft.Spark.E2ETest/IpcTests/JvmThreadPoolGCTests.cs

src/csharp/Microsoft.Spark/Interop/Ipc/JvmThreadPoolGC.cs

imback82 · 2020-10-04T03:22:45Z

src/scala/microsoft-spark-2.3.x/src/main/scala/org/apache/spark/api/dotnet/ThreadPool.scala

+  def run(threadId: Int, task: () => Unit): Unit = {
+    val executor = getOrCreateExecutor(threadId)
+    val future = executor.submit(new Runnable {
+      override def run(): Unit = task()


thanks for verifying this!

…hread_local

imback82

One nit comment, but LGTM, thanks @AFFogarty!

imback82 · 2020-10-05T22:13:09Z

src/csharp/Microsoft.Spark/Interop/Ipc/JvmThreadPoolGC.cs

+                }
+            }
+
+            _loggerService.LogDebug("JVM thread GC complete.");


Can we remove this log and line 130? (We don't have a good logger service, so this will always be printed). I think line 111 should be good enough.

Removed. Also fixed a bug where that function was always returning false.

AFFogarty added 11 commits August 24, 2020 21:13

Embed thread ID in payload

fdc1b20

Basic execution threadpool

88befdb

Thread lifecycle management

d9a403c

Port to spark 2.3

33c0202

Concurrent dictionary for active threads

a074028

Logic to clean up expired threads

6756f13

Copy changes to Spark 2.3

c875e63

Update ThreadPool in 2.3

3e37ca3

Merge branch 'master' of https://github.com/dotnet/spark into anfog/t…

3c81a14

…hread_local

Tests should pass now

e8aa36b

Merge branch 'master' of https://github.com/dotnet/spark into anfog/t…

9cd83df

…hread_local

imback82 assigned AFFogarty Sep 12, 2020

imback82 added the enhancement New feature or request label Sep 12, 2020

imback82 added this to the 1.0.0 milestone Sep 12, 2020

suhsteve mentioned this pull request Sep 15, 2020

Spark 3.0 readiness part 1 #647

Merged

7 tasks

Andrew Fogarty added 7 commits September 21, 2020 13:44

Added Spark 3 and fixed thread waiting

3942429

Merge branch 'master' of https://github.com/dotnet/spark into anfog/t…

1fff263

…hread_local

Fixed imports

b86eb00

Clean up ThreadPool

4eceb5d

Add ActiveSession APIs

e0f8d41

Refactor JvmThreadPool into separate class

e31fb00

Clean-up

1784d92

AFFogarty marked this pull request as ready for review September 22, 2020 22:44

AFFogarty changed the title ~~[WIP] Full support for multithreaded applications~~ Full support for multithreaded applications Sep 22, 2020

suhsteve reviewed Sep 23, 2020

View reviewed changes

suhsteve reviewed Sep 24, 2020

View reviewed changes

AFFogarty and others added 3 commits September 24, 2020 16:56

Update src/scala/microsoft-spark-2.3.x/src/main/scala/org/apache/spar…

5c45f2e

…k/api/dotnet/ThreadPool.scala Co-authored-by: Steve Suh <[email protected]>

Merge branch 'master' of https://github.com/dotnet/spark into anfog/t…

757bae9

…hread_local

Make executors private

6202f57

suhsteve mentioned this pull request Sep 24, 2020

[FEATURE REQUEST]: Spark 3.0 Readiness #633

Closed

61 tasks

Merge branch 'master' into anfog/thread_local

27f7d72

suhsteve previously approved these changes Oct 2, 2020

View reviewed changes

Merge branch 'master' into anfog/thread_local

53de2c0

AFFogarty requested review from Niharikadutta and imback82 October 2, 2020 17:14

Andrew Fogarty added 2 commits October 2, 2020 17:05

Merge branch 'master' of https://github.com/dotnet/spark into anfog/t…

8c55804

…hread_local

Merge branch 'anfog/thread_local' of https://github.com/AFFogarty/spark…

15840af

… into anfog/thread_local

Niharikadutta reviewed Oct 3, 2020

View reviewed changes

imback82 and others added 2 commits October 2, 2020 22:26

Merge branch 'master' into anfog/thread_local

606d3f6

Merge branch 'master' into anfog/thread_local

4c695b5

imback82 reviewed Oct 3, 2020

View reviewed changes

Address some of Terry's comments.

70404d4

AFFogarty dismissed suhsteve’s stale review via 70404d4 October 3, 2020 18:49

Andrew Fogarty added 2 commits October 3, 2020 11:50

Merge branch 'master' of https://github.com/dotnet/spark into anfog/t…

dcd05b9

…hread_local

Fix test name

7c585c8

AFFogarty requested review from imback82 and suhsteve October 3, 2020 19:07

Merge branch 'master' into anfog/thread_local

59fbf1e

imback82 reviewed Oct 4, 2020

View reviewed changes

Andrew Fogarty added 3 commits October 4, 2020 15:53

Deleting thread returns bool

80fa44b

Address comments

1529a27

Merge branch 'master' of https://github.com/dotnet/spark into anfog/t…

e217942

…hread_local

AFFogarty requested a review from imback82 October 5, 2020 20:14

imback82 previously approved these changes Oct 5, 2020

View reviewed changes

Fix return value and clean up logs

789a555

AFFogarty dismissed imback82’s stale review via 789a555 October 5, 2020 22:52

AFFogarty requested a review from imback82 October 5, 2020 23:32

imback82 approved these changes Oct 6, 2020

View reviewed changes

imback82 merged commit a67ad59 into dotnet:master Oct 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Full support for multithreaded applications #641

Full support for multithreaded applications #641

AFFogarty commented Aug 25, 2020 •

edited by suhsteve

Loading

Niharikadutta Oct 3, 2020

Niharikadutta Oct 3, 2020

Niharikadutta commented Oct 3, 2020

imback82 left a comment

imback82 Oct 4, 2020

imback82 left a comment

imback82 Oct 5, 2020

AFFogarty Oct 5, 2020

Full support for multithreaded applications #641

Full support for multithreaded applications #641

Conversation

AFFogarty commented Aug 25, 2020 • edited by suhsteve Loading

Design

Additional features

Testing

Limitations

Niharikadutta Oct 3, 2020

Choose a reason for hiding this comment

Niharikadutta Oct 3, 2020

Choose a reason for hiding this comment

Niharikadutta commented Oct 3, 2020

imback82 left a comment

Choose a reason for hiding this comment

imback82 Oct 4, 2020

Choose a reason for hiding this comment

imback82 left a comment

Choose a reason for hiding this comment

imback82 Oct 5, 2020

Choose a reason for hiding this comment

AFFogarty Oct 5, 2020

Choose a reason for hiding this comment

AFFogarty commented Aug 25, 2020 •

edited by suhsteve

Loading