Re: Review Request: XMLPS should be able to stream large results
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3543/#review5227 --- Ship it! After testing I believe this is good to go. trunk/grid/src/main/java/org/apache/oodt/grid/ProductQueryServlet.java https://reviews.apache.org/r/3543/#comment11427 I remember receiving partial responses. Also the servlet API says to flush the output stream to commit the response. This is especially needed since we don't know the length of the response. http://tomcat.apache.org/tomcat-5.5-doc/servletapi/javax/servlet/ServletResponse.html#getOutputStream() trunk/grid/src/main/java/org/apache/oodt/grid/ProductQueryServlet.java https://reviews.apache.org/r/3543/#comment11428 Quoted from servlet API here http://tomcat.apache.org/tomcat-5.5-doc/servletapi/javax/servlet/http/HttpServlet.html#doGet(javax.servlet.http.HttpServletRequest,%20javax.servlet.http.HttpServletResponse): Where possible, set the Content-Length header (with the ServletResponse.setContentLength(int) method), to allow the servlet container to use a persistent connection to return its response to the client, improving performance. The content length is automatically set if the entire response fits inside the response buffer. When using HTTP 1.1 chunked encoding (which means that the response has a Transfer-Encoding header), do not set the Content-Length header. - Thus, we don't set Content-Length and always use chunked Transfer-Encoding. Tested using cURL to watch the header values. Tomcat 5.5.35 : works Tomcat 6.0.35: works Tomcat 7.0.25: works trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/DBMSExecutor.java https://reviews.apache.org/r/3543/#comment11425 it is organized. OODT first, JDK second, removed unused imports. trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/XMLPSProductHandler.java https://reviews.apache.org/r/3543/#comment11426 yep organized. - Ricky On 2012-01-19 02:54:44, Ricky Nguyen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3543/ --- (Updated 2012-01-19 02:54:44) Review request for oodt. Summary --- See OODT-341: https://issues.apache.org/jira/browse/OODT-341 * CDEResult extends org.apache.oodt.xmlquery.Result * CDEResult mimetype is always text/plain * CDEResult size is always 0 * CDEResult inputstream is CDEResultInputStream * CDEResultInputStream has IO methods and wraps a CDEResult * CDEResult wraps a ResultSet and returns rows as Strings, applying MappingFuncs if a Mapping is provided, and appending constant fields if a List of CDEValues is provided. * ProductQueryServlet relies on servlet container to handle Content-Length (and possibly Transfer-Encoding: chunked) This addresses bug OODT-341. https://issues.apache.org/jira/browse/OODT-341 Diffs - trunk/grid/src/main/java/org/apache/oodt/grid/ProductQueryServlet.java 1183564 trunk/xmlps/pom.xml 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/DBMSExecutor.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/XMLPSProductHandler.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/structs/CDEResult.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/structs/CDEResultInputStream.java PRE-CREATION trunk/xmlps/src/test/java/org/apache/oodt/xmlps/structs/TestCDEResult.java PRE-CREATION trunk/xmlps/src/test/java/org/apache/oodt/xmlps/structs/TestCDEResultInputStream.java PRE-CREATION Diff: https://reviews.apache.org/r/3543/diff Testing --- Runs in Tomcat 7. I've used it at CHLA. NOT tested in other app servers. Which other app servers should I test? NOT tested MappingFuncs. Is it necessary? Added 2 unit tests: TestCDEResult and TestCDEResultInputStream, which both pass. Thanks, Ricky
Re: Review Request: XMLPS should be able to stream large results
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3543/#review5230 --- Ship it! Looks good! Ship it! - Chris On 2012-01-19 02:54:44, Ricky Nguyen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3543/ --- (Updated 2012-01-19 02:54:44) Review request for oodt. Summary --- See OODT-341: https://issues.apache.org/jira/browse/OODT-341 * CDEResult extends org.apache.oodt.xmlquery.Result * CDEResult mimetype is always text/plain * CDEResult size is always 0 * CDEResult inputstream is CDEResultInputStream * CDEResultInputStream has IO methods and wraps a CDEResult * CDEResult wraps a ResultSet and returns rows as Strings, applying MappingFuncs if a Mapping is provided, and appending constant fields if a List of CDEValues is provided. * ProductQueryServlet relies on servlet container to handle Content-Length (and possibly Transfer-Encoding: chunked) This addresses bug OODT-341. https://issues.apache.org/jira/browse/OODT-341 Diffs - trunk/grid/src/main/java/org/apache/oodt/grid/ProductQueryServlet.java 1183564 trunk/xmlps/pom.xml 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/DBMSExecutor.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/XMLPSProductHandler.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/structs/CDEResult.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/structs/CDEResultInputStream.java PRE-CREATION trunk/xmlps/src/test/java/org/apache/oodt/xmlps/structs/TestCDEResult.java PRE-CREATION trunk/xmlps/src/test/java/org/apache/oodt/xmlps/structs/TestCDEResultInputStream.java PRE-CREATION Diff: https://reviews.apache.org/r/3543/diff Testing --- Runs in Tomcat 7. I've used it at CHLA. NOT tested in other app servers. Which other app servers should I test? NOT tested MappingFuncs. Is it necessary? Added 2 unit tests: TestCDEResult and TestCDEResultInputStream, which both pass. Thanks, Ricky
Re: Review Request: XMLPS should be able to stream large results
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3543/ --- (Updated 2012-02-21 04:09:58.286025) Review request for oodt. Changes --- CDEResult.getSize() overridden to return -1. ProductQueryServlet only outputs Content-Length header when Result.getSize() = 0, in case other implementations of ProductHandler return Results that shouldn't be chunked. This is safer than always using chunked Transfer-Encoding (since the behavior is changed only for CDEResults). Summary --- See OODT-341: https://issues.apache.org/jira/browse/OODT-341 * CDEResult extends org.apache.oodt.xmlquery.Result * CDEResult mimetype is always text/plain * CDEResult size is always 0 * CDEResult inputstream is CDEResultInputStream * CDEResultInputStream has IO methods and wraps a CDEResult * CDEResult wraps a ResultSet and returns rows as Strings, applying MappingFuncs if a Mapping is provided, and appending constant fields if a List of CDEValues is provided. * ProductQueryServlet relies on servlet container to handle Content-Length (and possibly Transfer-Encoding: chunked) This addresses bug OODT-341. https://issues.apache.org/jira/browse/OODT-341 Diffs (updated) - trunk/grid/src/main/java/org/apache/oodt/grid/ProductQueryServlet.java 1291543 trunk/xmlps/pom.xml 1291543 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/DBMSExecutor.java 1291543 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/XMLPSProductHandler.java 1291543 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/structs/CDEResult.java 1291543 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/structs/CDEResultInputStream.java PRE-CREATION trunk/xmlps/src/test/java/org/apache/oodt/xmlps/structs/TestCDEResult.java PRE-CREATION trunk/xmlps/src/test/java/org/apache/oodt/xmlps/structs/TestCDEResultInputStream.java PRE-CREATION Diff: https://reviews.apache.org/r/3543/diff Testing --- Runs in Tomcat 7. I've used it at CHLA. NOT tested in other app servers. Which other app servers should I test? NOT tested MappingFuncs. Is it necessary? Added 2 unit tests: TestCDEResult and TestCDEResultInputStream, which both pass. Thanks, Ricky
Re: Review Request: XMLPS should be able to stream large results
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3543/#review4520 --- trunk/grid/src/main/java/org/apache/oodt/grid/ProductQueryServlet.java https://reviews.apache.org/r/3543/#comment10119 were you seeing an issue with web grid not flushing the result? trunk/grid/src/main/java/org/apache/oodt/grid/ProductQueryServlet.java https://reviews.apache.org/r/3543/#comment10120 Does Tomcat6 (or Tomcat5) set the container length? trunk/xmlps/pom.xml https://reviews.apache.org/r/3543/#comment10121 EasyMock ships under ALv2: http://easymock.org/ Nice Ricky. trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/DBMSExecutor.java https://reviews.apache.org/r/3543/#comment10122 anal retentive me -- can we keep the imports organized? *smile* trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/XMLPSProductHandler.java https://reviews.apache.org/r/3543/#comment10123 imports organized *smile* - Chris On 2012-01-19 02:54:44, Ricky Nguyen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3543/ --- (Updated 2012-01-19 02:54:44) Review request for oodt. Summary --- See OODT-341: https://issues.apache.org/jira/browse/OODT-341 * CDEResult extends org.apache.oodt.xmlquery.Result * CDEResult mimetype is always text/plain * CDEResult size is always 0 * CDEResult inputstream is CDEResultInputStream * CDEResultInputStream has IO methods and wraps a CDEResult * CDEResult wraps a ResultSet and returns rows as Strings, applying MappingFuncs if a Mapping is provided, and appending constant fields if a List of CDEValues is provided. * ProductQueryServlet relies on servlet container to handle Content-Length (and possibly Transfer-Encoding: chunked) This addresses bug OODT-341. https://issues.apache.org/jira/browse/OODT-341 Diffs - trunk/grid/src/main/java/org/apache/oodt/grid/ProductQueryServlet.java 1183564 trunk/xmlps/pom.xml 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/DBMSExecutor.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/XMLPSProductHandler.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/structs/CDEResult.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/structs/CDEResultInputStream.java PRE-CREATION trunk/xmlps/src/test/java/org/apache/oodt/xmlps/structs/TestCDEResult.java PRE-CREATION trunk/xmlps/src/test/java/org/apache/oodt/xmlps/structs/TestCDEResultInputStream.java PRE-CREATION Diff: https://reviews.apache.org/r/3543/diff Testing --- Runs in Tomcat 7. I've used it at CHLA. NOT tested in other app servers. Which other app servers should I test? NOT tested MappingFuncs. Is it necessary? Added 2 unit tests: TestCDEResult and TestCDEResultInputStream, which both pass. Thanks, Ricky
Re: Review Request: XMLPS should be able to stream large results
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3543/#review4521 --- Ship it! Ricky great job. Some minor nits, nothing that is a blocker. Review em' and if you have time would appreciate addressing them but if not, I'm not going to stand in your way. +1. - Chris On 2012-01-19 02:54:44, Ricky Nguyen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3543/ --- (Updated 2012-01-19 02:54:44) Review request for oodt. Summary --- See OODT-341: https://issues.apache.org/jira/browse/OODT-341 * CDEResult extends org.apache.oodt.xmlquery.Result * CDEResult mimetype is always text/plain * CDEResult size is always 0 * CDEResult inputstream is CDEResultInputStream * CDEResultInputStream has IO methods and wraps a CDEResult * CDEResult wraps a ResultSet and returns rows as Strings, applying MappingFuncs if a Mapping is provided, and appending constant fields if a List of CDEValues is provided. * ProductQueryServlet relies on servlet container to handle Content-Length (and possibly Transfer-Encoding: chunked) This addresses bug OODT-341. https://issues.apache.org/jira/browse/OODT-341 Diffs - trunk/grid/src/main/java/org/apache/oodt/grid/ProductQueryServlet.java 1183564 trunk/xmlps/pom.xml 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/DBMSExecutor.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/XMLPSProductHandler.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/structs/CDEResult.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/structs/CDEResultInputStream.java PRE-CREATION trunk/xmlps/src/test/java/org/apache/oodt/xmlps/structs/TestCDEResult.java PRE-CREATION trunk/xmlps/src/test/java/org/apache/oodt/xmlps/structs/TestCDEResultInputStream.java PRE-CREATION Diff: https://reviews.apache.org/r/3543/diff Testing --- Runs in Tomcat 7. I've used it at CHLA. NOT tested in other app servers. Which other app servers should I test? NOT tested MappingFuncs. Is it necessary? Added 2 unit tests: TestCDEResult and TestCDEResultInputStream, which both pass. Thanks, Ricky
Re: Review Request: XMLPS should be able to stream large results
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3543/ --- (Updated 2012-01-19 02:54:44.310091) Review request for oodt. Summary --- See OODT-341: https://issues.apache.org/jira/browse/OODT-341 * CDEResult extends org.apache.oodt.xmlquery.Result * CDEResult mimetype is always text/plain * CDEResult size is always 0 * CDEResult inputstream is CDEResultInputStream * CDEResultInputStream has IO methods and wraps a CDEResult * CDEResult wraps a ResultSet and returns rows as Strings, applying MappingFuncs if a Mapping is provided, and appending constant fields if a List of CDEValues is provided. * ProductQueryServlet relies on servlet container to handle Content-Length (and possibly Transfer-Encoding: chunked) This addresses bug OODT-341. https://issues.apache.org/jira/browse/OODT-341 Diffs - trunk/grid/src/main/java/org/apache/oodt/grid/ProductQueryServlet.java 1183564 trunk/xmlps/pom.xml 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/DBMSExecutor.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/XMLPSProductHandler.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/structs/CDEResult.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/structs/CDEResultInputStream.java PRE-CREATION trunk/xmlps/src/test/java/org/apache/oodt/xmlps/structs/TestCDEResult.java PRE-CREATION trunk/xmlps/src/test/java/org/apache/oodt/xmlps/structs/TestCDEResultInputStream.java PRE-CREATION Diff: https://reviews.apache.org/r/3543/diff Testing (updated) --- Runs in Tomcat 7. I've used it at CHLA. NOT tested in other app servers. Which other app servers should I test? NOT tested MappingFuncs. Is it necessary? Added 2 unit tests: TestCDEResult and TestCDEResultInputStream, which both pass. Thanks, Ricky
Re: Review Request: XMLPS should be able to stream large results
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3543/#review4462 --- trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/DBMSExecutor.java https://reviews.apache.org/r/3543/#comment10013 The MappingFuncs are applied in CDEResult#applyMappingFuncs() trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/XMLPSProductHandler.java https://reviews.apache.org/r/3543/#comment10014 Line 272 (latest) corresponds to line 250 (patched). trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/XMLPSProductHandler.java https://reviews.apache.org/r/3543/#comment10015 Appending constant values to the end of the row is now done in CDEResult#addConstValues() - Ricky On 2012-01-19 02:25:16, Ricky Nguyen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3543/ --- (Updated 2012-01-19 02:25:16) Review request for oodt. Summary --- See OODT-341: https://issues.apache.org/jira/browse/OODT-341 * CDEResult extends org.apache.oodt.xmlquery.Result * CDEResult mimetype is always text/plain * CDEResult size is always 0 * CDEResult inputstream is CDEResultInputStream * CDEResultInputStream has IO methods and wraps a CDEResult * CDEResult wraps a ResultSet and returns rows as Strings, applying MappingFuncs if a Mapping is provided, and appending constant fields if a List of CDEValues is provided. * ProductQueryServlet relies on servlet container to handle Content-Length (and possibly Transfer-Encoding: chunked) This addresses bug OODT-341. https://issues.apache.org/jira/browse/OODT-341 Diffs - trunk/grid/src/main/java/org/apache/oodt/grid/ProductQueryServlet.java 1183564 trunk/xmlps/pom.xml 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/DBMSExecutor.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/product/XMLPSProductHandler.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/structs/CDEResult.java 1233127 trunk/xmlps/src/main/java/org/apache/oodt/xmlps/structs/CDEResultInputStream.java PRE-CREATION trunk/xmlps/src/test/java/org/apache/oodt/xmlps/structs/TestCDEResult.java PRE-CREATION trunk/xmlps/src/test/java/org/apache/oodt/xmlps/structs/TestCDEResultInputStream.java PRE-CREATION Diff: https://reviews.apache.org/r/3543/diff Testing --- Runs in Tomcat 7. I've used it at CHLA. NOT tested in other app servers. Added 2 unit tests: TestCDEResult and TestCDEResultInputStream, which both pass. Thanks, Ricky
