Skip to content

Conversation

Hisoka-X
Copy link
Member

@Hisoka-X Hisoka-X commented Sep 27, 2023

What changes were proposed in this pull request?

This PR remove LevelDB support

Why are the changes needed?

The leveldb project seems to be no longer maintained, and we can always replace it with rocksdb.

Does this PR introduce any user-facing change?

Yes, the LevelDB will not supported in 4.1.0

How was this patch tested?

exist test.

Was this patch authored or co-authored using generative AI tooling?

No

@Hisoka-X
Copy link
Member Author

cc @dongjoon-hyun @LuciferYang

.version("3.4.0")
.stringConf
.transform(_.toUpperCase(Locale.ROOT))
.checkValues(DBBackend.values.map(_.toString).toSet)
.createWithDefault(DBBackend.LEVELDB.name)
.createWithDefault(DBBackend.ROCKSDB.name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not against this change, but I'm quite conflicted about it.

For shuffle db, we didn't make ROCKSDB the default in Spark 3.5.0, so I'm not sure if it's appropriate to just remove it in Spark 4.0. This is why I haven't submitted a pr to address this issue.

Let's wait for other people's opinions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, let us change to rocksdb as default first.
We can always remove it in 4.1 or later

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, let me change to rocksdb as default with another PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hisoka-X Spark 4.0 has already been released. Are you still interested in this work?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, let me clean it up again.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @LuciferYang , I did some update to this PR. Could you help to reopen it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

As indicated in the above prompt, it seems that I'm unable to reopen this pr either.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I created another pr #51027

@Hisoka-X Hisoka-X marked this pull request as draft September 27, 2023 04:55
@LuciferYang
Copy link
Contributor

@Hisoka-X Can you restart this job? I believe we can still remove Leveldb's support for HybridStore in Spark 4.0.

Copy link

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@github-actions github-actions bot added the Stale label Jan 13, 2024
@github-actions github-actions bot closed this Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants