Remove JDK 1.8 specific functions from tikv-client

frew commented 1 year ago

What problem does this PR solve?

Removes JDK 1.8 specific code.

What is changed and how it works?

Mostly the change doesn't affect functionality - we pull in javax.annotation-api from Maven which should work for both JDK 1.8 and later versions and remove unused code in MemoryUtil.

The one functionality change is removing MemoryUtil.free calls from TiBlockColumnVector and AutoGrowByteBuffer. This will defer buffer removal from the close() call to the finalize() call on the relevant object, but there doesn't appear to be a clean way independent of JDK versions to do this. I've tested some with Spark and it doesn't appear to have a major impact on memory usage.

Check List

Tests

Manual test (add detailed scripts or steps below) Most changes are verified by compilation passing. Memory changes have been verified by manually running some Spark jobs.

Code changes

Has exported function/method change Some MemoryUtil methods have been removed, but that's intended to be internal.

Side effects

Possible performance regression

Possible that memory usage will change due to free() call removal.

Related changes

None of the above

ti-chi-bot[bot] commented 1 year ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please assign humengyu2012 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/pingcap/tispark/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

sre-bot commented 1 year ago

All committers have signed the CLA.

ti-chi-bot[bot] commented 1 year ago

Welcome @frew! It looks like this is your first PR to pingcap/tispark 🎉

xuanyu66 commented 1 year ago

Currently, we don't have a plan to support JDK(>1.8), it's a huge work.

frew commented 1 year ago

@xuanyu66 Apologies if I'm missing something, but it seems like this PR makes it work? I've at least compiled TiSpark on JDK 11 with this PR and used it to export >10 TB of data successfully. Am I missing a use case that requires the unsafe free code?

xuanyu66 commented 1 year ago

@frew Thanks for contributing. We will keep an eye on this and find time to do a thorough test

shiyuhang0 commented 1 year ago

/run-all-tests

pingcap / tispark