Hi,
lately I have been trying to track down a memory leak found in
one of our GeoServer instances. The memory dump clearly showed
thousands of file name held by a map that backs the File.deleteOnExit
functionality. I found a couple of spots in non test code in GeoServer
that contained such call and removed it, yet I could not figure out
how they were relevant to our instance: one was in the FileStrategy,
whilst the other was in the WMS GetMap post code, but only if one
used a special request flag to force request validation.
The second was actually never used, and for the first, well,
we had SPEED strategy configured, so that did not make sense.
Until... I found out that the code choosing the strategy
always uses the FILE strategy when "verbose exceptions" is turned
on, see:
https://svn.codehaus.org/geoserver/branches/2.0.x/src/main/src/main/java/org/vfny/geoserver/servlets/ServiceStrategyFactory.java
line 73.
Maybe it's just me, but that choice seems overkill. The fact that
I want full stack traces in verbose exceptions does not mean that
I also want to write the full output to a file before sending it
out.
I mean, they definitely interact, writing out to a file guarantees
that in case of error a stack trace will be returned (since we did
not clobber the servlet output stream), but a BufferedOutuputStrategy
should be good enough in most cases, no?
What I'd like to do is:
- to remove that path and make the output strategy independent of
the verbose exception setting
- maybe expose the service strategy in the GUI to make it easier
to change (and most importantly, to make it modifiable at runtime
if one needs to).
Opinions?
Cheers
Andrea
--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.
Sounds reasonable to me. Can't quite remember why that was done with verbose exceptions... might have been before my time.
Andrea Aime wrote:
Hi,
lately I have been trying to track down a memory leak found in
one of our GeoServer instances. The memory dump clearly showed
thousands of file name held by a map that backs the File.deleteOnExit
functionality. I found a couple of spots in non test code in GeoServer
that contained such call and removed it, yet I could not figure out
how they were relevant to our instance: one was in the FileStrategy,
whilst the other was in the WMS GetMap post code, but only if one
used a special request flag to force request validation.
The second was actually never used, and for the first, well,
we had SPEED strategy configured, so that did not make sense.
Until... I found out that the code choosing the strategy
always uses the FILE strategy when "verbose exceptions" is turned
on, see:
https://svn.codehaus.org/geoserver/branches/2.0.x/src/main/src/main/java/org/vfny/geoserver/servlets/ServiceStrategyFactory.java
line 73.
Maybe it's just me, but that choice seems overkill. The fact that
I want full stack traces in verbose exceptions does not mean that
I also want to write the full output to a file before sending it
out.
I mean, they definitely interact, writing out to a file guarantees
that in case of error a stack trace will be returned (since we did
not clobber the servlet output stream), but a BufferedOutuputStrategy
should be good enough in most cases, no?
What I'd like to do is:
- to remove that path and make the output strategy independent of
the verbose exception setting
- maybe expose the service strategy in the GUI to make it easier
to change (and most importantly, to make it modifiable at runtime
if one needs to).
Opinions?
Cheers
Andrea
--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
Sounds reasonable to me as well.
I would add one unrelated thing. If my memory serves me right we use a
ByteArrayOutputStream for buffering the answer. Should we switch to
using something like a FastBufferedOutputStream (there exist thousands
of implementation on the net) for squeezing some more performance?
Simone.
-------------------------------------------------------
Ing. Simone Giannecchini
GeoSolutions S.A.S.
Founder - Software Engineer
Via Carignoni 51
55041 Camaiore (LU)
Italy
phone: +39 0584983027
fax: +39 0584983027
mob: +39 333 8128928
http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.linkedin.com/in/simonegiannecchini
http://twitter.com/simogeo
-------------------------------------------------------
On Sun, Jan 17, 2010 at 7:03 AM, Justin Deoliveira <jdeolive@anonymised.com> wrote:
Sounds reasonable to me. Can't quite remember why that was done with
verbose exceptions... might have been before my time.
Andrea Aime wrote:
Hi,
lately I have been trying to track down a memory leak found in
one of our GeoServer instances. The memory dump clearly showed
thousands of file name held by a map that backs the File.deleteOnExit
functionality. I found a couple of spots in non test code in GeoServer
that contained such call and removed it, yet I could not figure out
how they were relevant to our instance: one was in the FileStrategy,
whilst the other was in the WMS GetMap post code, but only if one
used a special request flag to force request validation.
The second was actually never used, and for the first, well,
we had SPEED strategy configured, so that did not make sense.
Until... I found out that the code choosing the strategy
always uses the FILE strategy when "verbose exceptions" is turned
on, see:
https://svn.codehaus.org/geoserver/branches/2.0.x/src/main/src/main/java/org/vfny/geoserver/servlets/ServiceStrategyFactory.java
line 73.
Maybe it's just me, but that choice seems overkill. The fact that
I want full stack traces in verbose exceptions does not mean that
I also want to write the full output to a file before sending it
out.
I mean, they definitely interact, writing out to a file guarantees
that in case of error a stack trace will be returned (since we did
not clobber the servlet output stream), but a BufferedOutuputStrategy
should be good enough in most cases, no?
What I'd like to do is:
- to remove that path and make the output strategy independent of
the verbose exception setting
- maybe expose the service strategy in the GUI to make it easier
to change (and most importantly, to make it modifiable at runtime
if one needs to).
Opinions?
Cheers
Andrea
--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Simone Giannecchini ha scritto:
Sounds reasonable to me as well.
I would add one unrelated thing. If my memory serves me right we use a
ByteArrayOutputStream for buffering the answer. Should we switch to
using something like a FastBufferedOutputStream (there exist thousands
of implementation on the net) for squeezing some more performance?
Looked around, but those classes won't do us any good. They usually
work by building a linked list of byte arrays or byte buffers instead
of reallocating and expanding just one.
However, in our PartialBufferedOutputStream2 we allocate a ByteArrayOutputStream already sized at the max size we'll use for
partial buffering, once we reach that we flush it and drop the
ByteArrayOutputStream, writing directly to the servlet output
instead.
Long story short, the byte internal to the ByteArrayOutputStream
gets never reallocated and resized.
Cheers
Andrea
--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.