lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Uwe Schindler (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (SOLR-12290) Do not close any servlet streams and improve our servlet stream closing prevention code for users and devs.
Date Sun, 06 May 2018 17:50:00 GMT

    [ https://issues.apache.org/jira/browse/SOLR-12290?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16465235#comment-16465235
] 

Uwe Schindler edited comment on SOLR-12290 at 5/6/18 5:49 PM:
--------------------------------------------------------------

This commit breaks TestCSVLoader on Windows. It looks like Solr no longer closes any content
streams that are passed separately to the servlet stream:

{noformat}
   [junit4]   2> NOTE: reproduce with: ant test  -Dtestcase=TestCSVLoader -Dtests.method=testLiteral
-Dtests.seed=B40869AE03F63CBC -Dtests.locale=ms -Dtests.timezone=ECT -Dtests.asserts=true
-Dtests.file.encoding=UTF-8
   [junit4] ERROR   0.06s | TestCSVLoader.testLiteral <<<
   [junit4]    > Throwable #1: java.nio.file.FileSystemException: C:\Users\Uwe Schindler\Projects\lucene\trunk-lusolr1\solr\build\solr-core\test\J0\temp\solr.handler.TestCSVLoader_B40869AE03F63CBC-001\TestCSVLoader-006\solr_tmp.csv:
Der Prozess kann nicht auf die Datei zugreifen, da sie von einem anderen Prozess verwendet
wird.
   [junit4]    >        at __randomizedtesting.SeedInfo.seed([B40869AE03F63CBC:B2ACAF677D87833E]:0)
   [junit4]    >        at sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:86)
   [junit4]    >        at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:97)
   [junit4]    >        at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:102)
   [junit4]    >        at sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:269)
   [junit4]    >        at sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:103)
   [junit4]    >        at java.nio.file.Files.delete(Files.java:1126)
   [junit4]    >        at org.apache.solr.handler.TestCSVLoader.tearDown(TestCSVLoader.java:64)
   [junit4]    >        at java.lang.Thread.run(Thread.java:748)
{noformat}

I am not sure behind the reasoning of forcefully preventing closing of ServletInputStreams
- but IMHO, the better way to handle this is (we had this discussion already a while ago):

Wrap the ServletInput/ServletOutput streams with a CloseShieldXxxxStream and only pass this
one around. This allows that code anywhere in solr is free to close any streams correctly
(which it should), but the ServletStreams are kept open by the shield.
The issue we have no is that ContentStreams are not necessarily (like in BlobUploadHandler,
CSVHandler) coming from the servlet streams. If it is a file or a uploaded file, then we HAVE
to close the stream.

The reason behind everything here is a bug in Jetty - in contrast to Tomcat and all other
servlet containers, it closes the socket after you close the servlet streams. This is a bug
- sorry! Jetty should prevent closing the underlying stream stream!

Please revert the current commit. I can help in solving this correctly - unfortunately I am
on travel next week.


was (Author: thetaphi):
This commit breaks TestCSVLoader on Windows. It looks like Solr no longer closes any content
streams that are passed separately to the servlet stream:

{noformat}
   [junit4]   2> NOTE: reproduce with: ant test  -Dtestcase=TestCSVLoader -Dtests.method=testLiteral
-Dtests.seed=B40869AE03F63CBC -Dtests.locale=ms -Dtests.timezone=ECT -Dtests.asserts=true
-Dtests.file.encoding=UTF-8
   [junit4] ERROR   0.06s | TestCSVLoader.testLiteral <<<
   [junit4]    > Throwable #1: java.nio.file.FileSystemException: C:\Users\Uwe Schindler\Projects\lucene\trunk-lusolr1\solr\build\solr-core\test\J0\temp\solr.handler.TestCSVLoader_B40869AE03F63CBC-001\TestCSVLoader-006\solr_tmp.csv:
Der Prozess kann nicht auf die Datei zugreifen, da sie von einem anderen Prozess verwendet
wird.
   [junit4]    >        at __randomizedtesting.SeedInfo.seed([B40869AE03F63CBC:B2ACAF677D87833E]:0)
   [junit4]    >        at sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:86)
   [junit4]    >        at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:97)
   [junit4]    >        at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:102)
   [junit4]    >        at sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:269)
   [junit4]    >        at sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:103)
   [junit4]    >        at java.nio.file.Files.delete(Files.java:1126)
   [junit4]    >        at org.apache.solr.handler.TestCSVLoader.tearDown(TestCSVLoader.java:64)
   [junit4]    >        at java.lang.Thread.run(Thread.java:748)
{noformat}

I am not sure behind the reasoning of forcefully preventing closing of ServletInputStreams
- but IMHO, the better way to handle this is (we had this discussion already a while ago):

Wrap the ServletInput/ServletOutput streams with a CloseShieldXxxxStream and only pass this
one around. This allows that code anywhere in solr is free to close any streams correctly
(which it should), but the ServletStreams are kept open by the shield.
The issue we have no is that ContentStreams are not necessarily (like in BlobUploadHandler,
CSVHandler) coming from the servlet streams. If it is a file or a uploaded file, then we HAVE
to close the stream.

The reason behind everything here is a bug in Jetty - in contrast to Tomcat and all other
servlet containers, it closes the socket after you close the servlet streams. This is a bug
- sorry! Jetty should prevent closing the stream!

Please revert the current commit. I can help in solving this correctly - unfortunately I am
on travel next week.

> Do not close any servlet streams and improve our servlet stream closing prevention code
for users and devs.
> -----------------------------------------------------------------------------------------------------------
>
>                 Key: SOLR-12290
>                 URL: https://issues.apache.org/jira/browse/SOLR-12290
>             Project: Solr
>          Issue Type: Bug
>      Security Level: Public(Default Security Level. Issues are Public) 
>            Reporter: Mark Miller
>            Assignee: Mark Miller
>            Priority: Major
>             Fix For: 7.4, master (8.0)
>
>         Attachments: SOLR-12290.patch, SOLR-12290.patch, SOLR-12290.patch, SOLR-12290.patch
>
>
> Original Summary:
> When you fetch a file for replication we close the request output stream after writing
the file which ruins the connection for reuse.
> We can't close response output streams, we need to reuse these connections. If we do
close them, clients are hit with connection problems when they try and reuse the connection
from their pool.
> New Summary:
> At some point the above was addressed during refactoring. We should remove these neutered
closes and review our close shield code.
> If you are here to track down why this is done:
> Connection reuse requires that we read all streams and do not close them - instead the
container itself must manage request and response streams. If we allow them to be closed,
not only do we lose some connection reuse, but we can cause spurious client errors that can
cause expensive recoveries for no reason. The spec allows us to count on the container to
manage streams. It's our job simply to not close them and to always read them fully, from
client and server. 
> Java itself can help with always reading the streams fully up to some small default amount
of unread stream slack, but that is very dangerous to count on, so we always manually eat
up anything on the streams our normal logic ends up not reading for whatever reason.
> We also cannot call abort without ruining the connection or sendError. These should be
options of very last resort (requiring a blood sacrifice) or when shutting down.
>  
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message