apache / incubator-gluten

Gluten is a middle layer responsible for offloading JVM-based SQL engines' execution to native engines.
https://gluten.apache.org/
Apache License 2.0
1.22k stars 439 forks source link

There are two classes with the same name GlutenSQLAppStatusListener in the project,is it necessary to integrate the code together? #4419

Open Ivankings opened 10 months ago

Ivankings commented 10 months ago

Discussed in https://github.com/oap-project/gluten/discussions/4418

Originally posted by **Ivankings** January 16, 2024 There are two classes with the same name GlutenSQLAppStatusListener in the project, one in the gluten-core module and the other in the gluten-ui module, and these two classes have similar functions. From a design perspective, is it necessary to integrate the code together? For example, integrating into gluten-core? image
PHILO-HE commented 10 months ago

@ulysses-you

ulysses-you commented 10 months ago

It seems the GlutenSQLAppStatusListener in gluten-core is only used for CH backend, we can probably move it to CH backend. The GlutenSQLAppStatusListener in gluten-ui is for all backends. We can rename one of these two listeners if there is a conflict.

I do not think we should merge these two listeners into one. The original idea of GlutenSQLAppStatusListener in gluten-ui is for spark ui and history server. For history server, we do not require gluten-core dependency and just put gluten-ui.jar to the it's classpath.

Ivankings commented 10 months ago

@ulysses-you I'm a bit confused, the functionality of class org.apache.spark.listener.GlutenSQLAppStatusListener in gluten-core appears to be specifically designed for RPC communication between driver and executor. Perhaps it can be applied to all the backends, including velox.

image
ulysses-you commented 10 months ago

@Ivankings if you read this class git history, you can find it is used for CH backend only.

Ivankings commented 10 months ago

All right. @ulysses-you ,appreciate it!