[SPARK-22340][PYTHON][FOLLOW-UP] Add a better message and improve documentation for pinned thread mode

[HyukjinKwon]

What changes were proposed in this pull request?

This PR proposes to show different warning message when the pinned thread mode is enabled:

When enabled:

PYSPARK_PIN_THREAD feature is enabled. However, note that it cannot inherit the local properties from the parent thread although it isolates each thread on PVM and JVM with its own local properties.
To work around this, you should manually copy and set the local properties from the parent thread to the child thread when you create another thread.

When disabled:

Currently, 'setLocalProperty' (set to local properties) with multiple threads does not properly work.
Internally threads on PVM and JVM are not synced, and JVM thread can be reused for multiple threads on PVM, which fails to isolate local properties for each thread on PVM.
To work around this, you can set PYSPARK_PIN_THREAD to true (see SPARK-22340). However, note that it cannot inherit the local properties from the parent thread although it isolates each thread on PVM and JVM with its own local properties.
To work around this, you should manually copy and set the local properties from the parent thread to the child thread when you create another thread.

Why are the changes needed?

Currently, it shows the same warning message regardless of PYSPARK_PIN_THREAD being set. In the warning message it says "you can set PYSPARK_PIN_THREAD to true ..." which is confusing.

Does this PR introduce any user-facing change?

Documentation and warning message as shown above.

How was this patch tested?

Manually tested.

$ PYSPARK_PIN_THREAD=true ./bin/pyspark

sc.setJobGroup("a", "b")

.../pyspark/util.py:141: UserWarning: PYSPARK_PIN_THREAD feature is enabled. However, note that it cannot inherit the local properties from the parent thread although it isolates each thread on PVM and JVM with its own local properties.
To work around this, you should manually copy and set the local properties from the parent thread to the child thread when you create another thread.
  warnings.warn(msg, UserWarning)

$ ./bin/pyspark

sc.setJobGroup("a", "b")

.../pyspark/util.py:141: UserWarning: Currently, 'setJobGroup' (set to local properties) with multiple threads does not properly work.
Internally threads on PVM and JVM are not synced, and JVM thread can be reused for multiple threads on PVM, which fails to isolate local properties for each thread on PVM.
To work around this, you can set PYSPARK_PIN_THREAD to true (see SPARK-22340). However, note that it cannot inherit the local properties from the parent thread although it isolates each thread on PVM and JVM with its own local properties.
To work around this, you should manually copy and set the local properties from the parent thread to the child thread when you create another thread.
  warnings.warn(msg, UserWarning)

[HyukjinKwon]

cc @squito and @WeichenXu123

[SparkQA]

Test build #114044 has finished for PR 26588 at commit cfc1277.

• This patch fails PySpark unit tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
• Workaround class can be written as below (unofficial way):
• >>> class CustomThread(threading.Thread):
• Workaround class can be written as below (unofficial way):
• >>> class CustomThread(threading.Thread):
• Workaround class can be written as below (unofficial way):
• >>> class CustomThread(threading.Thread):

[SparkQA]

Test build #114050 has finished for PR 26588 at commit f0b1cc3.

• This patch fails PySpark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[HyukjinKwon]

retest this please

[SparkQA]

Test build #114054 has finished for PR 26588 at commit f0b1cc3.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[squito]

I like improving the error msg.

I'm not so sure about including the extra msg explaining the custom thread code, I have a feeling its more confusing than useful. I feel like users often won't even be creating threads themselves -- eg. they're running an http server which has a thread pool in it somewhere, but the developer doesn't even see it. It seems much more straightforward to just reset the relevant properties in each thread.

Maybe I'm not properly understanding how folks use the inheritable properties?

[HyukjinKwon]

Hm, right. I just realised that the use cases can be different. In our case, it needs to launch multiple jobs in multiple threads with each group id. Later, in the main thread, it cancels the jobs by the job group id.

Let me just remove the workaround codes. I think I agree that it might more cause confusions.

[SparkQA]

Test build #114182 has finished for PR 26588 at commit 5a4c142.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[HyukjinKwon]

Let me merge this one as is. I think it's now pretty conservative.

[squito]

👍