PECOBLR-1121 Arrow patch to circumvent Arrow issues with JDk 16+. #1156
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Patch Arrow to create a Databricks ArrowBuf which allocates memory on the heap and provides access to it through Java methods. This removes the need to specify "--add-opens=java.base/java.nio=ALL-UNNAMED" as JVM args for JDK 16+.
Use native Arrow if available. Otherwise fallback to the patch version.
Remove irrelevant reference counting in patch code. Patch code uses heap memory for arrow operations and reference counting is not required.
Remove redundant todos for accounting.
Patch DecimalUtility to not use unsafe methods to set decimal values on DatabricksArrowBuf.
Add notice to all patched Arrow Java code. In NOTICE file mention Arrow has been patched by Databricks.
On static init failure of MemoryUtil class, it prints a stack trace to stderr. Remove this print, since now we fallback to DatabricksBufferAllocator when this happens. And the error is logged as well.
tejassp-db
requested review from
gopalldb,
jayantsing-db,
jprakash-db,
madhav-db,
msrathore-db,
samikshya-db and
vikrantpuppala
|
|
||
| // ---- Databricks patch start ---- | ||
| private final HistoricalLog historicalLog = | ||
| DEBUG ? new HistoricalLog(DEBUG_LOG_LENGTH, "ArrowBuf[%d]", id) : null; |
Collaborator
There was a problem hiding this comment.
do we even need to worry about the historical log? can we not just set it to null? seems like its usage is null checked everywhere anyway?
Collaborator
There was a problem hiding this comment.
+1, add a comment on why this is needed
Comment on lines
+49
to
+54
| long currentReservation = reservedSize.get(); | ||
| long newReservation = currentReservation + nBytes; | ||
| if (newReservation > allocator.getHeadroom() + currentReservation) { | ||
| return false; | ||
| } | ||
| reservedSize.addAndGet(nBytes); |
Collaborator
There was a problem hiding this comment.
this is not thread safe, should we use compareAndSet?
Comment on lines
+114
to
+122
| @Override | ||
| public ReferenceManager getReferenceManager() { | ||
| return referenceManager; | ||
| } | ||
|
|
||
| @Override | ||
| public long capacity() { | ||
| return capacity; | ||
| } |
Collaborator
There was a problem hiding this comment.
i think a bunch of these methods do not have a changed override behaviour, can we remove them so that it is easy to review and maintain?
| if (capacity > Integer.MAX_VALUE) { | ||
| throw new IllegalArgumentException( | ||
| "DatabricksArrowBuf does not support capacity > Integer.MAX_VALUE"); | ||
| } |
Collaborator
There was a problem hiding this comment.
- why this limit?
- this is missing from the other constructor
- can we reuse constructors
gopalldb
reviewed
Jan 5, 2026
| static { | ||
| RootAllocator rootAllocator = null; | ||
| try { | ||
| rootAllocator = new RootAllocator(); |
Collaborator
There was a problem hiding this comment.
we were using Integer.MAX_VALUE, not needed now?
gopalldb
reviewed
Jan 5, 2026
| // ---- to avoid unsafe allocation initialization errors. | ||
| public static final String DEBUG_ALLOCATOR = "arrow.memory.debug.allocator"; | ||
| public static final int DEBUG_LOG_LENGTH = 6; | ||
| public static final boolean DEBUG; |
Collaborator
There was a problem hiding this comment.
nit: rename to better name than debug?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Databricks server shares query results in Arrow format for easy cross language functionality. The JDBC driver experiences compatibility issues with JDK 16 and later versions when processing Arrow results.
This problem arises from stricter encapsulation of internal APIs in newer Java versions, which affects the driver's use of the Apache Arrow result format consumption with the Apache Arrow library. The JDBC driver is used in partner solutions, where they do not have control of the runtime environment, and the workaround of setting JVM arguments is not feasible.
Testing
Tests are added in other stacked PRs.
Additional Notes to the Reviewer
Its a stacked PR.