You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2022/10/12 10:54:20 UTC
[GitHub] [druid] abhishekagarwal87 opened a new pull request, #13217: Add more information to exceptions occurred while writing temporary data
abhishekagarwal87 opened a new pull request, #13217:
URL: https://github.com/apache/druid/pull/13217
This typical error is seen when ingestion is writing intermediate data to a temporary file
```
java.io.IOException: No space left on device
at sun.nio.ch.FileDispatcherImpl.write0(Native Method) ~[?:?]
at sun.nio.ch.FileDispatcherImpl.write(FileDispatcherImpl.java:62) ~[?:?]
at sun.nio.ch.IOUtil.writeFromNativeBuffer(IOUtil.java:113) ~[?:?]
at sun.nio.ch.IOUtil.write(IOUtil.java:79) ~[?:?]
at sun.nio.ch.FileChannelImpl.write(FileChannelImpl.java:280) ~[?:?]
at org.apache.druid.io.Channels.writeFully(Channels.java:32) ~[?:?]
at org.apache.druid.segment.writeout.FileWriteOutBytes.flush(FileWriteOutBytes.java:62) ~[?:?]
at org.apache.druid.segment.writeout.FileWriteOutBytes.flushIfNeeded(FileWriteOutBytes.java:54) ~[?:?]
at org.apache.druid.segment.writeout.FileWriteOutBytes.write(FileWriteOutBytes.java:86) ~[?:?]
at org.apache.druid.io.Channels.writeFully(Channels.java:32) ~[?:?]
```
From this exception, it's hard to know how much data we were trying to write or what location we were exactly trying to write to. I added a bit of code to intercept this exception and add some more info to the outbound exception.
This PR has:
- [ ] been self-reviewed.
- [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm merged pull request #13217: Add more information to exceptions occurred while writing temporary data
Posted by GitBox <gi...@apache.org>.
gianm merged PR #13217:
URL: https://github.com/apache/druid/pull/13217
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhishekagarwal87 commented on pull request #13217: Add more information to exceptions occurred while writing temporary data
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on PR #13217:
URL: https://github.com/apache/druid/pull/13217#issuecomment-1276016646
I see your point. Maybe “space needed for this file” or “current size of
the file”?
On Wed, 12 Oct 2022 at 4:47 PM, Laksh Singla ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In
> processing/src/main/java/org/apache/druid/segment/writeout/FileWriteOutBytes.java
> <https://github.com/apache/druid/pull/13217#discussion_r993325018>:
>
> > @@ -59,7 +60,12 @@ private void flushIfNeeded(int bytesNeeded) throws IOException
> public void flush() throws IOException
> {
> buffer.flip();
> - Channels.writeFully(ch, buffer);
> + try {
> + Channels.writeFully(ch, buffer);
> + }
> + catch (IOException e) {
> + throw new IOE(e, "Failed to write to file: %s. Space needed: %d", file.getAbsolutePath(), writeOutBytes);
>
> From the exception, it seems like that the writeOutBytes is the space
> deficit due to which the ingestion is failing, whereas the actual deficit
> for the job to complete can be much more than that right?
> Should the error message omit the information Space needed: %d or be
> reworded such that the user knows that writeOutBytes more are required to
> complete the current operation (and not the ingestion)?
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/druid/pull/13217#pullrequestreview-1138901445>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AALIWUOVZA64VZPCG2UZSHDWC2M6NANCNFSM6AAAAAARDFGFPA>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] LakshSingla commented on pull request #13217: Add more information to exceptions occurred while writing temporary data
Posted by GitBox <gi...@apache.org>.
LakshSingla commented on PR #13217:
URL: https://github.com/apache/druid/pull/13217#issuecomment-1276032256
`Current size of the file: %d` seems like a better alternative to me.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] LakshSingla commented on a diff in pull request #13217: Add more information to exceptions occurred while writing temporary data
Posted by GitBox <gi...@apache.org>.
LakshSingla commented on code in PR #13217:
URL: https://github.com/apache/druid/pull/13217#discussion_r993325018
##########
processing/src/main/java/org/apache/druid/segment/writeout/FileWriteOutBytes.java:
##########
@@ -59,7 +60,12 @@ private void flushIfNeeded(int bytesNeeded) throws IOException
public void flush() throws IOException
{
buffer.flip();
- Channels.writeFully(ch, buffer);
+ try {
+ Channels.writeFully(ch, buffer);
+ }
+ catch (IOException e) {
+ throw new IOE(e, "Failed to write to file: %s. Space needed: %d", file.getAbsolutePath(), writeOutBytes);
Review Comment:
From the exception, it seems like that the `writeOutBytes` is the space deficit due to which the ingestion is failing, whereas the actual deficit for the job to complete can be much more than that right?
Should the error message omit the information `Space needed: %d` or be reworded such that the user knows that `writeOutBytes` more are required to complete the current operation (and not the ingestion)?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] cryptoe commented on a diff in pull request #13217: Add more information to exceptions occurred while writing temporary data
Posted by GitBox <gi...@apache.org>.
cryptoe commented on code in PR #13217:
URL: https://github.com/apache/druid/pull/13217#discussion_r993411373
##########
processing/src/main/java/org/apache/druid/segment/writeout/FileWriteOutBytes.java:
##########
@@ -59,7 +60,12 @@ private void flushIfNeeded(int bytesNeeded) throws IOException
public void flush() throws IOException
{
buffer.flip();
- Channels.writeFully(ch, buffer);
+ try {
+ Channels.writeFully(ch, buffer);
+ }
+ catch (IOException e) {
+ throw new IOE(e, "Failed to write to file: %s. Space needed: %d", file.getAbsolutePath(), writeOutBytes);
Review Comment:
I think we can't guess how much many more bytes are needed as that is dependent on when the user will call the close 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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhishekagarwal87 commented on pull request #13217: Add more information to exceptions occurred while writing temporary data
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on PR #13217:
URL: https://github.com/apache/druid/pull/13217#issuecomment-1276124762
@LakshSingla @cryptoe - thanks for reviewing. I have made the changes.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org