Extend DiskManager to manager other temp files like temp shuffle IPC files in Ballista, cached data files etc

[mingmwang]

Is your feature request related to a problem or challenge? Please describe what you are trying to do. A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] (This section helps Arrow developers understand the context and why for this feature, in addition to the what)

Describe the solution you'd like A clear and concise description of what you want to happen.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context Add any other context or screenshots about the feature request here.

[mingmwang]

https://github.com/apache/arrow-ballista/issues/496

https://github.com/apache/arrow-ballista/issues/558

[mingmwang]

@alamb @tustvold @andygrove @yahoNanJing I would like to extend the current DiskManager's functionality to not only manager the temp spill files but also the shuffle IPC files in Ballista, as well as the Cached data files generated by the System.

The current DiskManager is only used to hold the temp directories to store the temporary files generated by any spillable MemoryConsumers. The DiskManager might be disabled in case that we do not want the MemoryConsumer to spill even under memory pressure.

But if I extend the capabilities of the DiskManager, it means it should never be disabled. the spilling disable logic will have to be moved out from the DiskManager.

I'm not sure whether I should extend the current DiskManager's capabilities or keep it unchanged and add another new PersistFileManager. Want to hear your opinions on this.

[tustvold]

I'm not sure whether I should extend the current DiskManager's capabilities or keep it unchanged and add another new PersistFileManager.

Given shuffling IPC files seems specific to Ballista, it would perhaps make more sense to be something that lives within that codebase? So a ShuffleManager or PersistFileManager as you suggest

[alamb]

Given shuffling IPC files seems specific to Ballista, it would perhaps make more sense to be something that lives within that codebase? So a ShuffleManager or PersistFileManager as you suggest

One reason it might make sense to extend DiskManager is so that Ballista / DataFusion could have a more complete view of disk file usage. For example, if Ballista wanted to be able to cap the total disk space used across IPC files and spill files it might be hard to do so with two different disk managers.

But if I extend the capabilities of the DiskManager, it means it should never be disabled. the spilling disable logic will have to be moved out from the DiskManager.

I suspect you could extend the API to distinguish between "is spilling allowed" and "please keep track of and clean up these files made during execution" as well

I don't have a strong preference how you do it as long as there is still a way to limit memory used by SortExec -- there is a test for this in https://github.com/apache/arrow-datafusion/blob/master/datafusion/core/tests/memory_limit.rs so that should help guide the implementation

[tustvold]

One reason it might make sense to extend DiskManager is so that Ballista / DataFusion could have a more complete view of disk file usage

Perhaps composition might be the solution here, i.e. a SpillManager that uses DiskManager under the hood

[mingmwang]

@tustvold @alamb Thanks for your suggestion. I will try to add a different PersistFileManager in Ballista first to manage the temp shuffle files and temp cache files. After that, we can go further and merge the PersistFileManager functionality into DataFusion's DiskManager and the ultimate goal is to have one DiskManager which can have the complete view of disk usage, temp file management, working dir/disk health check etc.