[GLUTEN-7837][VL] Spark driver should not initialize cache if not in local mode

[leoluan2009]

What changes were proposed in this pull request?

Fixes: #7837

Spark driver should not initialize cache if not in local mode

How was this patch tested?

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)

(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

[github-actions]

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/apache/incubator-gluten/issues

Then could you also rename commit message and pull request title in the following format?

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

• Other pull requests

[leoluan2009]

@zhztheplayer can you help review?

[github-actions]

#7837

[zhztheplayer]

@leoluan2009 Does this solve the cache issue mentioned in #7837?

[leoluan2009]

@leoluan2009 Does this solve the cache issue mentioned in #7837?

Yes, if VeloxBackend is running in driver, it will not init cache.

[zhztheplayer]

How does the change work with Spark local mode? I mean we may need to turn the cache on in local mode if user doesn't explicitly disable it.

[zhztheplayer]

Could just change from the caller code, something like:

if (!isDriver_) {
  initCache();
}
...

Would be enough and more readable.

[leoluan2009]

Done

[zhztheplayer]

Hi @leoluan2009 , as spark.gluten.sql.columnar.backend.velox.cacheEnabled is now a static SQL conf, can we just modify the config map passed to native to set it to false from non-local driver side? It's a workaround and we can remove once a optimal design is out.

[leoluan2009]

Hi @leoluan2009 , as spark.gluten.sql.columnar.backend.velox.cacheEnabled is now a static SQL conf, can we just modify the config map passed to native to set it to false from non-local driver side? It's a workaround and we can remove once a optimal design is out.

it's ok and I will update the pr

[leoluan2009]

@zhztheplayer I have updated this PR, please help to review, thanks!

[zhztheplayer]

Would you like to add some comments in code? Or just leave a link to the relevant issue/PR. Thanks.

[leoluan2009]

Done, thanks

[zhztheplayer]

👍

[leoluan2009]

@zhztheplayer resolved all comments

[FelixYBW]

@zhztheplayer @leoluan2009
can you go through all the initializations there? Is there any other things we shouldn't run on driver? Is the local cache only exception? I'd expect the initialization should be much different.

If we have more than one, we should considered to seperate the initialization routine for driver and worker.

[zhztheplayer]

can you go through all the initializations there? Is there any other things we shouldn't run on driver? Is the local cache only exception? I'd expect the initialization should be much different.

Not much difference so far and it's also about our C++ API's encapsulation. We can make trade-offs once it's really needed.

[zhztheplayer]

If we have more than one, we should considered to seperate the initialization routine for driver and worker.

In future there will be different ways to implement differentiated initialization, but exposing Spark arch conceptions "driver" / "executor" / "task" to C++ code could be the last ones to consider. We can use configurations, or compose finer-grained JNI initialization APIs, etc.

[FelixYBW]

In future there will be different ways to implement differentiated initialization, but exposing Spark arch conceptions "driver" / "executor" / "task" to C++ code could be the last ones to consider. We can use configurations, or compose finer-grained JNI initialization APIs, etc.

Let's re-evaluate the mythology once we have another exception.