[GLUEN-10107] Introduce NeedCustomColumnarBatchSerializer trait to make columnarBatchSerializerClass custom by rss implementation

[zjuwangg]

What changes were proposed in this pull request?

(Fixes: #10107)

How was this patch tested?

Existing tests.

[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

[github-actions]

Run Gluten Clickhouse CI on x86

[zjuwangg]

@zhztheplayer Do you think it's proper way to decouple rss related config from common module?

[zhztheplayer]

Thanks for working on this @zjuwangg.

The solution looks promising, except that it adds some layers of complexity (e.g., more reflections) which needs extra effort from reader / user. I may need to have a deeper look to understand the cost of refactoring the existing code. Let me know if you have relevant insights.

[zjuwangg]

Thanks for working on this @zjuwangg.

The solution looks promising, except that it adds some layers of complexity (e.g., more reflections) which needs extra effort from reader / user. I may need to have a deeper look to understand the cost of refactoring the existing code. Let me know if you have relevant insights.

@zhztheplayer
Sure, it indeed introduces complexity. Now we have gluten-common/ backend-specific-impl / rss-specific-impl, and backend depends on gluten-common, while rss-specific-impl depends on backends.
Current design VeloxCelebornColumnarBatchSerlizerFactory works just like the way of VeloxCelebornColumnarShuffleWriterFactory does. Besides that, we can also leverage CelebornShuffleWriterFactory interface, add the columnarBatchSeriliazer interface and change the interface name to reduce reflections.

In the long run, we may also can extract RSS-related configuration and behavior like backend. We may need introduce CelebornShuffleBackend and UniffleShuffleBackend and pass current job shuffle backend to execution-backend to do more specific backend-related and rss-related config.

[zhztheplayer]

Hi @zjuwangg, sorry for the delayed response. Are you going to modify GlutenConfig in this PR?

[zjuwangg]

Hi @zjuwangg, sorry for the delayed response. Are you going to modify GlutenConfig in this PR?
@zhztheplayer

No, It's just a poc of my idea. I plan to modify related config in following PR.

[zjuwangg]

@zhztheplayer
Currently, RSS-specific configurations exist beyond just createColumnarBatchSerializer (e.g., this genShuffleWriterType). We should introduce more middle interfaces to decouple these tightly bound dependencies.

[github-actions]

Run Gluten Clickhouse CI on x86

[zhztheplayer]

@zjuwangg CI is failing

[zjuwangg]

@zjuwangg CI is failing

Got. Will address it soon.

[github-actions]

Run Gluten Clickhouse CI on x86

[github-actions]

Run Gluten Clickhouse CI on x86

[github-actions]

Run Gluten Clickhouse CI on x86

[github-actions]

Run Gluten Clickhouse CI on x86

[github-actions]

Run Gluten Clickhouse CI on x86

[github-actions]

Run Gluten Clickhouse CI on x86

[zhztheplayer]

Thanks.

[zjuwangg]

@zhztheplayer Can you help merge this one?
I plan to change ck backend use this way to decouple the rss related config.

[zhztheplayer]

Thanks.