You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2020/07/10 13:06:22 UTC
[GitHub] [hive] szlta opened a new pull request #1238: HIVE-23824 - Initial commit
szlta opened a new pull request #1238:
URL: https://github.com/apache/hive/pull/1238
## NOTICE
Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HIVE-XXXXX: Fix a typo in YYY)
For more details, please see https://cwiki.apache.org/confluence/display/Hive/HowToContribute
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] szlta commented on a change in pull request #1238: HIVE-23824
Posted by GitBox <gi...@apache.org>.
szlta commented on a change in pull request #1238:
URL: https://github.com/apache/hive/pull/1238#discussion_r453588505
##########
File path: llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/OrcEncodedDataReader.java
##########
@@ -575,31 +626,8 @@ private OrcFileMetadata getFileFooterFromCacheOrDisk() throws IOException {
tailBuffers = metadataCache.getFileMetadata(fileKey);
if (tailBuffers != null) {
try {
- MemoryBuffer tailBuffer = tailBuffers.getSingleBuffer();
- ByteBuffer bb = null;
- if (tailBuffer != null) {
- bb = tailBuffer.getByteBufferDup();
- // TODO: remove the copy after ORC-158 and ORC-197
- // if (bb.isDirect()) {
- ByteBuffer dupBb = tailBuffer.getByteBufferDup(); // Don't mess with the cached object.
- bb = ByteBuffer.allocate(dupBb.remaining());
- bb.put(dupBb);
- bb.flip();
- // }
- } else {
- // TODO: add the ability to extractFileTail to read from multiple buffers?
- MemoryBuffer[] tailBufferArray = tailBuffers.getMultipleBuffers();
- int totalSize = 0;
- for (MemoryBuffer buf : tailBufferArray) {
- totalSize += buf.getByteBufferRaw().remaining();
- }
- bb = ByteBuffer.allocate(totalSize);
- for (MemoryBuffer buf : tailBufferArray) {
- bb.put(buf.getByteBufferDup());
- }
- bb.flip();
- }
- OrcTail orcTail = ReaderImpl.extractFileTail(bb);
+ OrcTail orcTail = getOrcTailFromLlapBuffers(tailBuffers);
Review comment:
Yes, it won't be evicted as it is locked. The getFileMetadata invocation will incRef it (if it is found of course)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1238: HIVE-23824
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #1238:
URL: https://github.com/apache/hive/pull/1238#discussion_r453556055
##########
File path: llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/OrcEncodedDataReader.java
##########
@@ -575,31 +626,8 @@ private OrcFileMetadata getFileFooterFromCacheOrDisk() throws IOException {
tailBuffers = metadataCache.getFileMetadata(fileKey);
if (tailBuffers != null) {
try {
- MemoryBuffer tailBuffer = tailBuffers.getSingleBuffer();
- ByteBuffer bb = null;
- if (tailBuffer != null) {
- bb = tailBuffer.getByteBufferDup();
- // TODO: remove the copy after ORC-158 and ORC-197
- // if (bb.isDirect()) {
- ByteBuffer dupBb = tailBuffer.getByteBufferDup(); // Don't mess with the cached object.
- bb = ByteBuffer.allocate(dupBb.remaining());
- bb.put(dupBb);
- bb.flip();
- // }
- } else {
- // TODO: add the ability to extractFileTail to read from multiple buffers?
- MemoryBuffer[] tailBufferArray = tailBuffers.getMultipleBuffers();
- int totalSize = 0;
- for (MemoryBuffer buf : tailBufferArray) {
- totalSize += buf.getByteBufferRaw().remaining();
- }
- bb = ByteBuffer.allocate(totalSize);
- for (MemoryBuffer buf : tailBufferArray) {
- bb.put(buf.getByteBufferDup());
- }
- bb.flip();
- }
- OrcTail orcTail = ReaderImpl.extractFileTail(bb);
+ OrcTail orcTail = getOrcTailFromLlapBuffers(tailBuffers);
Review comment:
Oh, I see now... it is decRefBuffer...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1238: HIVE-23824
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #1238:
URL: https://github.com/apache/hive/pull/1238#discussion_r453555463
##########
File path: llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/OrcEncodedDataReader.java
##########
@@ -575,31 +626,8 @@ private OrcFileMetadata getFileFooterFromCacheOrDisk() throws IOException {
tailBuffers = metadataCache.getFileMetadata(fileKey);
if (tailBuffers != null) {
try {
- MemoryBuffer tailBuffer = tailBuffers.getSingleBuffer();
- ByteBuffer bb = null;
- if (tailBuffer != null) {
- bb = tailBuffer.getByteBufferDup();
- // TODO: remove the copy after ORC-158 and ORC-197
- // if (bb.isDirect()) {
- ByteBuffer dupBb = tailBuffer.getByteBufferDup(); // Don't mess with the cached object.
- bb = ByteBuffer.allocate(dupBb.remaining());
- bb.put(dupBb);
- bb.flip();
- // }
- } else {
- // TODO: add the ability to extractFileTail to read from multiple buffers?
- MemoryBuffer[] tailBufferArray = tailBuffers.getMultipleBuffers();
- int totalSize = 0;
- for (MemoryBuffer buf : tailBufferArray) {
- totalSize += buf.getByteBufferRaw().remaining();
- }
- bb = ByteBuffer.allocate(totalSize);
- for (MemoryBuffer buf : tailBufferArray) {
- bb.put(buf.getByteBufferDup());
- }
- bb.flip();
- }
- OrcTail orcTail = ReaderImpl.extractFileTail(bb);
+ OrcTail orcTail = getOrcTailFromLlapBuffers(tailBuffers);
Review comment:
What happens if the tail is evicted from the cache in the meantime (after the check, but before this line). Or it is locked during this code?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1238: HIVE-23824
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #1238:
URL: https://github.com/apache/hive/pull/1238#discussion_r453552118
##########
File path: llap-client/src/java/org/apache/hadoop/hive/llap/io/api/LlapIo.java
##########
@@ -35,6 +41,15 @@
*/
long purge();
+ /**
+ * Returns a deserialized OrcTail instance associated with the ORC file on the given path.
+ * Raw content is either obtained from cache, or from disk if there is a cache miss.
+ * @param path Orc file path
+ * @param conf jobConf
Review comment:
Missing javadoc for cachetag
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvary merged pull request #1238: HIVE-23824
Posted by GitBox <gi...@apache.org>.
pvary merged pull request #1238:
URL: https://github.com/apache/hive/pull/1238
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org