Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ballista reloaded - proposed changes to core ballista #1066

Closed
wants to merge 32 commits into from

Conversation

milenkovicm
Copy link
Contributor

Which issue does this PR close?

Closes #.

Rationale for this change

First of all, apologies for a gigantic PR, but I did not see better way of making this proposal.

With this proposal I'd like to propose one way forward for ballista, direction is not set in stone so we could discuss alternatives.

I'd like to set the stage with driving idea, it may help understanding PR better.

I believe if we want to revive ballista we need to make it a library rather than full fledge application with all batteries included. If we continue developing it as an application, core code will just grow, putting maintainers on pressure to maintain code which may not be related to core ballista mission.
Making ballista as library, which could bootstrap schedulers and executors with few lines of code, with possibility to plug in additional functionalities will keep use-case specific code out of ballista. We should make it as simple as bootstrapping datafusion application.

Second goal, I believe, should be keeping ballista functionalities on par with datafusion releases. At the moment ballista is missing a lot of functionalities, like writes, sinks, to name a few. The main, short term, goal would be to get writes, inserts supported.

Lastly, client-scheduler, and scheduler-executor interaction should be improved. Client-scheduler at the moment communicate using custom made gprc protocol, which could get support for UDF(s), configuration ... current client-scheduler protocol can be made an optional component, so users can bring their own frontend such as spark connect, or similar. scheduler-executor as well can be extended to support session configuration information, ...

So with those high level goals in mind, I have done a bit of code trimming and maintenance. I have removed some code which I personally believe should/could be an "add-on" to ballista.

What changes are included in this PR?

This PR removes:

  • python support (datafusion python is the main effort, we should focus making it run on ballista)
  • helm support (this can be contrib project)
  • good part of ci directory (ci works without it)
  • plugin sub-system (never finalized)
  • caching support (this can be a contrib project)
  • support for hdfs (library too old, there are newer library to be used)
  • UI (can be a contrib project)
  • made keda, rest-api, flight-sql optional (we could move them to contrib project)
  • key value store is gone, only memory store remains
  • removes some dependencies which were detected as not used

This PR adds no functionality, but updates most of the dependencies.

Are there any user-facing changes?

@milenkovicm
Copy link
Contributor Author

Will have something ready later today or tomorrow @andygrove, I was thinking to put proposal of the next steps on top of this PR, started already but did not have time to finish it yet.

Now, this PR has many things removed, but not too many code changed, would you like to take it step by step and re-do this work again or we can just merge this as once off, without squashing commits so we can revert removal if needed @andygrove ? I understand that it is a bit of big PR.

This PR removes:

  • python support (datafusion python is the main effort, we should focus making it run on ballista)
  • helm support (this can be contrib project)
  • good part of ci directory (ci works without it)
  • plugin sub-system (never finalized)
  • caching support (this can be a contrib project)
  • support for hdfs (library too old, there are newer library to be used)
  • UI (can be a contrib project)
  • made keda, rest-api, flight-sql optional (we could move them to contrib project)
  • key value store is gone, only memory store remains
  • removes some dependencies which were detected as not used

@milenkovicm
Copy link
Contributor Author

also, we could remove deps update commits and cherry pick it to new PR

@andygrove
Copy link
Member

Now, this PR has many things removed, but not too many code changed, would you like to take it step by step and re-do this work again or we can just merge this as once off, without squashing commits so we can revert removal if needed @andygrove ? I understand that it is a bit of big PR.

Honestly, this is too big for me to do a comprehensive review within the limited weekend time that I have available. It seems that some very unrelated changes would be easy to split out, so we can start with those first to at least reduce the size of this PR?

For example, these could be separate smaller PRs that would be easy to review:

  • Remove UI
  • Remove Python
  • Remove Helm
  • Clean up CI
  • Clean up dependencies

We could get through these very quickly, I think.

@milenkovicm
Copy link
Contributor Author

all right, will cherry-pick issue from big branch. I'm doing this at my spare time so bear with me :)

created #1067 listing all features for removal, and #1068 as high level road map proposal to kick off discussions and ideas.

is it ok just to refer PR to #1067 list or you prefer having issue created for each PR @andygrove

@andygrove
Copy link
Member

all right, will cherry-pick issue from big branch. I'm doing this at my spare time so bear with me :)

created #1067 listing all features for removal, and #1068 as high level road map proposal to kick off discussions and ideas.

is it ok just to refer PR to #1067 list or you prefer having issue created for each PR @andygrove

That's great. Thank you. No need to create separate issues per item.

milenkovicm added a commit to milenkovicm/arrow-ballista that referenced this pull request Oct 11, 2024
milenkovicm added a commit to milenkovicm/arrow-ballista that referenced this pull request Oct 11, 2024
it looks like not used anywhere.

Relates to: apache#1066 & apache#1067
@milenkovicm milenkovicm mentioned this pull request Oct 11, 2024
milenkovicm added a commit to milenkovicm/arrow-ballista that referenced this pull request Oct 11, 2024
andygrove pushed a commit that referenced this pull request Oct 11, 2024
it looks like not used anywhere.

Relates to: #1066 & #1067
andygrove pushed a commit that referenced this pull request Oct 11, 2024
milenkovicm added a commit to milenkovicm/arrow-ballista that referenced this pull request Oct 12, 2024
milenkovicm added a commit to milenkovicm/arrow-ballista that referenced this pull request Oct 12, 2024
As it may be useful I'd argue that it should not be
part of core ballista. We see in the community
caches build on top of object_store.

If functionality like this is needed it should be
implemented as scheduler-policy or similar.

Relates to: apache#1066 & apache#1067
andygrove pushed a commit that referenced this pull request Oct 12, 2024
As it may be useful I'd argue that it should not be
part of core ballista. We see in the community
caches build on top of object_store.

If functionality like this is needed it should be
implemented as scheduler-policy or similar.

Relates to: #1066 & #1067
@milenkovicm
Copy link
Contributor Author

milenkovicm commented Oct 13, 2024

Thanks all for you comments, I'm closing this PR as most of changes have been done in #1067.
It would be greatly appreciated if you have a read and comment #1068, I'm going to break down task and start implementation once we wrap up #1067

@milenkovicm milenkovicm deleted the trim_some_weight branch November 26, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development-process documentation Improvements or additions to documentation python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants