You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2021/03/02 02:20:55 UTC
[GitHub] [drill] cgivre opened a new pull request #2184: DRILL-7874: Drill Fails to Read File Types on S3
cgivre opened a new pull request #2184:
URL: https://github.com/apache/drill/pull/2184
# [DRILL-7845](https://issues.apache.org/jira/browse/DRILL-7874): Drill Fails to Read File Types on S3
## Description
This PR addresses:
* https://issues.apache.org/jira/browse/DRILL-7874
* https://issues.apache.org/jira/browse/DRILL-7875
Full description in JIRA, but this PR switches the various format plugins to use the function `openDecompressedInputStream` rather than `open` or `openPossiblyDecompressedStream`. The latter was causing issues when querying files stored on s3.
DRILL-7875 adds a `null` check in the Splunk reader.
## Documentation
No user facing changes.
## Testing
Manual testing and existing unit tests.
----------------------------------------------------------------
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
[GitHub] [drill] cgivre commented on pull request #2184: DRILL-7875: Drill Fails to Splunk Indexes with no Timestamp
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2184:
URL: https://github.com/apache/drill/pull/2184#issuecomment-792188267
@vvysotskyi I fixed this PR to use the latest changes from your s3 fix. Now it just addresses the NPE from Splunk indexes w/o timestamps.
----------------------------------------------------------------
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
[GitHub] [drill] cgivre commented on a change in pull request #2184: DRILL-7874: Drill Fails to Read File Types on S3
Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2184:
URL: https://github.com/apache/drill/pull/2184#discussion_r585893757
##########
File path: contrib/format-esri/src/main/java/org/apache/drill/exec/store/esri/ShpBatchReader.java
##########
@@ -125,7 +125,7 @@ public boolean next() {
private void openFile(FileSchemaNegotiator negotiator) {
try {
- fileReaderShp = negotiator.fileSystem().open(split.getPath());
+ fileReaderShp = negotiator.fileSystem().openDecompressedInputStream(split.getPath());
Review comment:
@vvysotskyi
Thanks for your comment. The issue that was happening on S3 was that the `inputstream` that gets passed to the format plugin seemed to be compressed in some way, even if the file itself was not compressed.
This didn't seem to matter for text based file formats, but for formats that read binary data, such as `pcap`, the stream wasn't being decompressed and the result was that Drill couldn't parse the file.
The `openPossiblyCompressedStream()` method didn't really solve this issue, because you could end up with a compressed stream that the format plugins couldn't read. I thought another approach would be to put this logic in the format plugins themselves, but I couldn't figure out a way to determine whether the stream was compressed or not after you call the `openPossiblyCompressedStream()`.
Do you have any suggestions as to how to fix so that we can avoid the OOM?
----------------------------------------------------------------
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
[GitHub] [drill] vvysotskyi commented on pull request #2184: DRILL-7874: Drill Fails to Read Binary File Types on S3
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on pull request #2184:
URL: https://github.com/apache/drill/pull/2184#issuecomment-791907508
Hi @luocooong, no, this PR contains also fixes for the SHP and Splunk plugins (DRILL-7875) that aren't addressed in #2186. So those fixes may be left here, but other changes related to replacing `openPossiblyCompressedStream` with `openDecompressedInputStream` should be reverted since they don't fix the actual issue.
----------------------------------------------------------------
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
[GitHub] [drill] vvysotskyi commented on a change in pull request #2184: DRILL-7874: Drill Fails to Read File Types on S3
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2184:
URL: https://github.com/apache/drill/pull/2184#discussion_r585890334
##########
File path: contrib/format-esri/src/main/java/org/apache/drill/exec/store/esri/ShpBatchReader.java
##########
@@ -125,7 +125,7 @@ public boolean next() {
private void openFile(FileSchemaNegotiator negotiator) {
try {
- fileReaderShp = negotiator.fileSystem().open(split.getPath());
+ fileReaderShp = negotiator.fileSystem().openDecompressedInputStream(split.getPath());
Review comment:
I have some doubts regarding the safety of the `openDecompressedInputStream()` method. It contains the following line:
```
byte[] bytes = IOUtils.toByteArray(compressedStream);
```
that will read all the stream to the memory, but it may cause OOM.
Why we can't use here another `openPossiblyCompressedStream()` method?
----------------------------------------------------------------
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
[GitHub] [drill] vvysotskyi commented on a change in pull request #2184: DRILL-7874: Drill Fails to Read File Types on S3
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2184:
URL: https://github.com/apache/drill/pull/2184#discussion_r585908797
##########
File path: contrib/format-esri/src/main/java/org/apache/drill/exec/store/esri/ShpBatchReader.java
##########
@@ -125,7 +125,7 @@ public boolean next() {
private void openFile(FileSchemaNegotiator negotiator) {
try {
- fileReaderShp = negotiator.fileSystem().open(split.getPath());
+ fileReaderShp = negotiator.fileSystem().openDecompressedInputStream(split.getPath());
Review comment:
Both methods `openPossiblyCompressedStream()` and `openDecompressedInputStream()` have similar logic in determining compression codec (based on specified path) and producing decompression. The difference is only that the last one places whole the stream to the buffer and wraps it to the `ByteArrayInputStream`. So looks like if `openDecompressedInputStream()` will not help, the `openPossiblyCompressedStream()` also wouldn't work.
If for some reason files from s3 are read as compressed, and there is no way for obtaining info for the compression method when reading the file, we can add a property to the file storage plugin `config` block that will hold the default compression type if any.
----------------------------------------------------------------
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
[GitHub] [drill] cgivre merged pull request #2184: DRILL-7875: Drill Fails to Splunk Indexes with no Timestamp
Posted by GitBox <gi...@apache.org>.
cgivre merged pull request #2184:
URL: https://github.com/apache/drill/pull/2184
----------------------------------------------------------------
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