Hi all,
thanks Andrea and Jody for your comments.
Andrea, no, I'm not getting more than 1000 failures, nor I'm the rendering
is taking more than 60 seconds, but *you're absolutely right:
StreamingRenderer is indeed already lenient on errors*, as you explained. I
overlooked the fireErrorEvent() method, taking for granted that it would
end up stopping the rendering process at some point. I guess I was misled
by the fact that the JMapPane I was using to display the map would paint
the map just once and then refuse to do any further rendering.
I inspected the code a bit deeper and I think I figured out what was
happening:
1. an IllegalArgumentException was thrown by the underlying
JDBCFeatureReader when a feature with invalid geometry was found
2. the exception was caught by StreamingRenderer, which would then
invoke the fireErrorEvent() method to log it (SEVERE level) and to
propagate it to any registered RenderListener (invoking their
errorOccurred(Exception) method)
3. in this case, the registered listener was an instance of
RenderingTask, whose errorOccurred() implementation would set the failed
flag to true, causing the JMapPane's RenderingExecutor to stubbornly refuse
to start any further RenderingTask
I modified RenderingTask.errorOccurred() as such:
public void errorOccurred(Exception e) {
if (!(e instanceof IllegalArgumentException)) {
running.set(false);
failed.set(true);
}
}
...and now I see a flood of exceptions in the log, but rendering never
stops! :-)
I then set out to find any implementation of the RenderListener interface
in GeoTools/GeoServer to see what they do in their errorOccurred()
implementation, and I found an interesting one in GeoServer, WMS
module, org.geoserver.wms.map.RenderExceptionStrategy:
/**
* Upon a render exception check if its cause is one that we actually
want to ignore, and if
* not abort the rendering process so the map producer can fail.
*/
public void errorOccurred(final Exception renderException) {
Throwable cause = renderException;
while (cause != null) {
if (cause instanceof TransformException
|| cause instanceof IllegalAttributeException
|| cause instanceof FactoryException
|| cause instanceof NoninvertibleTransformException) {
if (LOGGER.isLoggable(Level.FINE)) {
LOGGER.log(Level.FINE, "Ignoring renderer error",
renderException);
}
return;
}
cause = cause.getCause();
}
// not an ignorable cause... stop rendering
LOGGER.log(Level.FINE, "Got an unexpected render exception.",
renderException);
this.renderException = renderException;
renderer.stopRendering();
}
As you can see, this implementation ignores a few exception types, among
which IllegalArgumentException is NOT present, and stops the renderer in
any other case. RenderExceptionStrategy is used
by org.geoserver.wms.map.RenderedImageMapOutputFormat,
org.geoserver.wms.map.PDFMapResponse
and org.geoserver.wms.svg.SVGBatikMapOutputFormat, so I guess for those
classes an invalid geometry would indeed abort rendering.
Anyway, the person who reported the issue doesn't say anything about the
context in which he sees that rendering is stopped (is it a web app calling
GeoServer's WMS service? is it a standalone Swing application using
JMapPane?), so I don't know if we're talking about RenderingTask or
RenderExceptionStrategy...
or some other RenderListener implementation I'm not aware of.
In any case, I'm still convinced that a more meaningful exception should be
thrown in case an invalid geometry is read and then implementations of
RenderListener will be able to make an informed decision on what to do with
it (basically, either ignore it or quit rendering). Testing if the
exception is an instance of IllegalArgumentException seems to me too prone
to error.
Any further thoughts?
Stefano
On Wed, Feb 11, 2015 at 8:44 PM, Andrea Aime <andrea.a...@geo-solutions.it>
wrote:
> Hi Stefano,
> sorry for the late reply, I'm stll stuck in bed with fever...
> Anyways, the streaming renderer is built to be tolerant to errors, the
> code looks as follows:
>
> try {
> painter.paint(graphic, shape, style, scale, labelObstacle);
> } catch(Throwable t) {
> fireErrorEvent(t);
> }
>
> So you can see each and any exception is caught and does not stop the
> rendering per se.
> What can stop the rendering are the listeners that get the error message,
> there
> is one in GeoServer, WMS module, RenderedImageMapOutputFormat class (going
> by
> memory, no IDE in front of me) that counts how many errors happened, and
> stops
> the renderer by calling stopRendering() when there are more than a admin
> configurable
> threshold.
>
> The default value for that threshold is 1000, so either you have more than
> 1000 failures,
> or something is not working as it should in the code that handles the
> error count.
> And oh, there is also a time based check, rendering is stopped by default
> after 60
> seconds, againt something tunable and happening in that GeoServer class,
> which
> you probably want to raise to a number more suitable to debugging.
>
> If there are indeed more than 1000 failures, maybe the fix should not be
> done here,
> but in the Oracle reader, recognizing that Oracle is happy to store broken
> geometries,
> and trying to do something to fix them before turning them into JTS
> geometries.
> In this case the class you want to look at is SDO, in the gt-jdbc-oracle
> module
>
> Hope this helps
>
> Cheers
> Andrea
>
>
> On Wed, Feb 11, 2015 at 6:38 PM, Stefano Costa <ridethepeng...@gmail.com>
> wrote:
>
>> Dear GeoTools developers,
>> I'm attempting to tackle issue GEOT-4885: GeoTools stops rendering valid
>> geometry elements after an invalid geometry element
>> <http://jira.codehaus.org/browse/GEOT-4885> and, since I'm pretty much a
>> newbie here, I need guidance as to how to solve it sensibly.
>>
>> The issue reporter laments that GeoTools stops rendering features in a
>> collection when an element with invalid geometry is found and provides
>> sample data to reproduce the error, which I did. In this particular case,
>> the culprits are several polygon geometries with less than 4 points: this
>> causes the
>> method com.vividsolutions.jts.geom.LinearRing.validateConstruction() to
>> throw a java.lang.IllegalArgumentException and the rendering process to
>> stop.
>>
>> I devised a very naive fix
>> patching org.geotools.renderer.lite.StreamingRenderer.drawPlain()
>> and org.geotools.renderer.lite.StreamingRenderer.drawOptimized() methods:
>> basically what I do is to catch the IllegalArgumentException, log it and
>> keep going with the rendering process. You can take a look at the code
>> here:
>> https://github.com/ridethepenguin/geotools/commit/0672a765d334018b3ec4bebd9d0d7d6cc0124dc2
>>
>> I realize this is a very naive approach, which makes the totally
>> unwarranted assumption that any IllegalArgumentException thrown by the code
>> inside the try-catch block is due to an invalid geometry being constructed.
>> A more sound approach would require, I believe, to have a custom exception
>> type (something along the lines of InvalidGeometryException) be thrown by
>> "geometry builders" and dealt with accordingly by client code.
>>
>> Now, how to do that sensibly? ;-)
>>
>> Proposals:
>>
>> 1. patch com.vividsolutions.jts.geom.GeometryFactory: I guess this
>> cannot/should not be done, as the class is part of the JTS library
>> 2. patch every single implementation of
>> org.geotools.data.FeatureReader: implies touching quite a few classes, but
>> I guess it's feasible
>> 3. ...any better ideas?
>>
>>
>> Thanks for your help,
>> Stefano Costa
>>
>>
>>
>> ------------------------------------------------------------------------------
>> Dive into the World of Parallel Programming. The Go Parallel Website,
>> sponsored by Intel and developed in partnership with Slashdot Media, is
>> your
>> hub for all things parallel software development, from weekly thought
>> leadership blogs to news, videos, case studies, tutorials and more. Take a
>> look and join the conversation now. http://goparallel.sourceforge.net/
>> _______________________________________________
>> GeoTools-Devel mailing list
>> GeoTools-Devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>>
>>
>
>
> --
> ==
> GeoServer Professional Services from the experts! Visit
> http://goo.gl/NWWaa2 for more information.
> ==
>
> Ing. Andrea Aime
> @geowolf
> Technical Lead
>
> GeoSolutions S.A.S.
> Via Poggio alle Viti 1187
> 55054 Massarosa (LU)
> Italy
> phone: +39 0584 962313
> fax: +39 0584 1660272
> mob: +39 339 8844549
>
> http://www.geo-solutions.it
> http://twitter.com/geosolutions_it
>
> *AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*
>
> Le informazioni contenute in questo messaggio di posta elettronica e/o
> nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il
> loro utilizzo è consentito esclusivamente al destinatario del messaggio,
> per le finalità indicate nel messaggio stesso. Qualora riceviate questo
> messaggio senza esserne il destinatario, Vi preghiamo cortesemente di
> darcene notizia via e-mail e di procedere alla distruzione del messaggio
> stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso,
> divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od
> utilizzarlo per finalità diverse, costituisce comportamento contrario ai
> principi dettati dal D.Lgs. 196/2003.
>
>
>
> The information in this message and/or attachments, is intended solely for
> the attention and use of the named addressee(s) and may be confidential or
> proprietary in nature or covered by the provisions of privacy act
> (Legislative Decree June, 30 2003, no.196 - Italy's New Data Protection
> Code).Any use not in accord with its purpose, any disclosure, reproduction,
> copying, distribution, or either dissemination, either whole or partial, is
> strictly forbidden except previous formal approval of the named
> addressee(s). If you are not the intended recipient, please contact
> immediately the sender by telephone, fax or e-mail and delete the
> information in this message that has been received in error. The sender
> does not give any warranty or accept liability as the content, accuracy or
> completeness of sent messages and accepts no responsibility for changes
> made after they were sent or for other risks which arise as a result of
> e-mail transmission, viruses, etc.
>
> -------------------------------------------------------
>
------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel
