[Geoserver-devel] issue with destroying datastores

Hi all,

When running unit tests I noticed that in eclipse they are extremely
slow. Digging into things I noticed that the catalog is read twice for
every unit tests setup/teardown cycle, whereas it should only be read once.

Digging into it I found out that the reason why is the destroy() method
on GeoServer. Which is called by spring on container shutdown. It grabs
an instance of "data" from the container and destroys all the datastores.

This is an issue because Data depends on GeoSrever, and not vice versa.
So the spring container actually destroys Data before destroying
GeoServer. Which means the call to
"applicationContext.getBean("catalog") is actually causing the container
to recreate the Data instance and hence loading the catalog again.

Anyways... long story short. I believe it would be better to also mark
Data as a "DisposableBean" (like GeoServer), and do the datastore
cleanup there.

Any objections to this. I have made the change and things now work as
expected with unit tests. The change was simple so I just went ahead an
committed it. IF there was a reason for the above behavior reverting my
change will be trivial.

Oddly enough unit tests in eclipse are still quite slow. And running all
the wfs tests for instance results in OOM errors. Seems there is a
another memory leak somewhere...

-Justin

--
Justin Deoliveira
The Open Planning Project
http://topp.openplans.org

Justin,

I'd noticed that geoserver was behaving this way generally (i.e. 'apply'
caused 'data' to reload twice), but never thought to look deeper into
it.

Glad you tracked it down and fixed it. Is this on trunk, or will you
backport to 1.6.x too?

--saul

On Sun, 2007-11-18 at 20:51 -0300, Justin Deoliveira wrote:

Hi all,

When running unit tests I noticed that in eclipse they are extremely
slow. Digging into things I noticed that the catalog is read twice for
every unit tests setup/teardown cycle, whereas it should only be read once.

Digging into it I found out that the reason why is the destroy() method
on GeoServer. Which is called by spring on container shutdown. It grabs
an instance of "data" from the container and destroys all the datastores.

This is an issue because Data depends on GeoSrever, and not vice versa.
So the spring container actually destroys Data before destroying
GeoServer. Which means the call to
"applicationContext.getBean("catalog") is actually causing the container
to recreate the Data instance and hence loading the catalog again.

Anyways... long story short. I believe it would be better to also mark
Data as a "DisposableBean" (like GeoServer), and do the datastore
cleanup there.

Any objections to this. I have made the change and things now work as
expected with unit tests. The change was simple so I just went ahead an
committed it. IF there was a reason for the above behavior reverting my
change will be trivial.

Oddly enough unit tests in eclipse are still quite slow. And running all
the wfs tests for instance results in OOM errors. Seems there is a
another memory leak somewhere...

-Justin

It sounds all good. I did the dispose on destroy thing but didn't realize the
impact on unit tests. There are no other reason to keep things there so doing
it in Data and making it a DisposableBean sounds just correct.

Oddly enough unit tests in eclipse are still quite slow. And running all
the wfs tests for instance results in OOM errors. Seems there is a
another memory leak somewhere...

You may try doing a one time data set up and tear down, at least for non
transactional tests, here's how to do that:
http://www.nabble.com/ArcSDE-built-times-jumped-up--tf4028147.html#a11493204

hope that helps,

Gabriel

-Justin

Cool. I agree with you gabriel, bringing the application context up and
down for every test is unnecessary. Perhaps we should give unit tests a
method for marking the data directory as dirty, and then recreate only
if the flag is set. Creating a jira for this.

http://jira.codehaus.org/browse/GEOS-1524

I have assigned it to me but if anyone wants to take it on please feel free.

-Justin

Gabriel Roldán wrote:

It sounds all good. I did the dispose on destroy thing but didn't realize the
impact on unit tests. There are no other reason to keep things there so doing
it in Data and making it a DisposableBean sounds just correct.

Oddly enough unit tests in eclipse are still quite slow. And running all
the wfs tests for instance results in OOM errors. Seems there is a
another memory leak somewhere...

You may try doing a one time data set up and tear down, at least for non
transactional tests, here's how to do that:
http://www.nabble.com/ArcSDE-built-times-jumped-up--tf4028147.html#a11493204

hope that helps,

Gabriel

-Justin

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

!DSPAM:4007,4741623725951637810514!

--
Justin Deoliveira
The Open Planning Project
http://topp.openplans.org

Yeah... it would be easy enough to backport to 1.6.x. Will throw that on
my todo list.

-Justin

Saul Farber wrote:

Justin,

I'd noticed that geoserver was behaving this way generally (i.e. 'apply'
caused 'data' to reload twice), but never thought to look deeper into
it.

Glad you tracked it down and fixed it. Is this on trunk, or will you
backport to 1.6.x too?

--saul

On Sun, 2007-11-18 at 20:51 -0300, Justin Deoliveira wrote:

Hi all,

When running unit tests I noticed that in eclipse they are extremely
slow. Digging into things I noticed that the catalog is read twice for
every unit tests setup/teardown cycle, whereas it should only be read once.

Digging into it I found out that the reason why is the destroy() method
on GeoServer. Which is called by spring on container shutdown. It grabs
an instance of "data" from the container and destroys all the datastores.

This is an issue because Data depends on GeoSrever, and not vice versa.
So the spring container actually destroys Data before destroying
GeoServer. Which means the call to
"applicationContext.getBean("catalog") is actually causing the container
to recreate the Data instance and hence loading the catalog again.

Anyways... long story short. I believe it would be better to also mark
Data as a "DisposableBean" (like GeoServer), and do the datastore
cleanup there.

Any objections to this. I have made the change and things now work as
expected with unit tests. The change was simple so I just went ahead an
committed it. IF there was a reason for the above behavior reverting my
change will be trivial.

Oddly enough unit tests in eclipse are still quite slow. And running all
the wfs tests for instance results in OOM errors. Seems there is a
another memory leak somewhere...

-Justin

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

!DSPAM:4007,47412644291993362379201!

--
Justin Deoliveira
The Open Planning Project
http://topp.openplans.org

Hi Justin,
I'm not sure if the extra complexity to mark the data dir as dirty is worth
the pain, provided that a per-test-suite setup and teardown can handle it
quite straightforwardly and for transaction tests it'll get dirty everytime?

Gabriel.

On Monday 19 November 2007 03:29:54 pm Justin Deoliveira wrote:

Cool. I agree with you gabriel, bringing the application context up and
down for every test is unnecessary. Perhaps we should give unit tests a
method for marking the data directory as dirty, and then recreate only
if the flag is set. Creating a jira for this.

http://jira.codehaus.org/browse/GEOS-1524

I have assigned it to me but if anyone wants to take it on please feel
free.

-Justin

Gabriel Roldán wrote:
> It sounds all good. I did the dispose on destroy thing but didn't realize
> the impact on unit tests. There are no other reason to keep things there
> so doing it in Data and making it a DisposableBean sounds just correct.
>
>> Oddly enough unit tests in eclipse are still quite slow. And running all
>> the wfs tests for instance results in OOM errors. Seems there is a
>> another memory leak somewhere...
>
> You may try doing a one time data set up and tear down, at least for non
> transactional tests, here's how to do that:
> http://www.nabble.com/ArcSDE-built-times-jumped-up--tf4028147.html#a11493
>204
>
> hope that helps,
>
> Gabriel
>
>> -Justin
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2005.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> _______________________________________________
> Geoserver-devel mailing list
> Geoserver-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/geoserver-devel
>
> !DSPAM:4007,4741623725951637810514!

Perhaps not... My rationale is for consistency of those writing
transaction tests. They have to worry about the order in which
transaction tests are executed, in order to determine what the data
should look like.

Unless I am missing something?

Gabriel Roldán wrote:

Hi Justin,
I'm not sure if the extra complexity to mark the data dir as dirty is worth
the pain, provided that a per-test-suite setup and teardown can handle it
quite straightforwardly and for transaction tests it'll get dirty everytime?

Gabriel.

On Monday 19 November 2007 03:29:54 pm Justin Deoliveira wrote:

Cool. I agree with you gabriel, bringing the application context up and
down for every test is unnecessary. Perhaps we should give unit tests a
method for marking the data directory as dirty, and then recreate only
if the flag is set. Creating a jira for this.

http://jira.codehaus.org/browse/GEOS-1524

I have assigned it to me but if anyone wants to take it on please feel
free.

-Justin

Gabriel Roldán wrote:

It sounds all good. I did the dispose on destroy thing but didn't realize
the impact on unit tests. There are no other reason to keep things there
so doing it in Data and making it a DisposableBean sounds just correct.

Oddly enough unit tests in eclipse are still quite slow. And running all
the wfs tests for instance results in OOM errors. Seems there is a
another memory leak somewhere...

You may try doing a one time data set up and tear down, at least for non
transactional tests, here's how to do that:
http://www.nabble.com/ArcSDE-built-times-jumped-up--tf4028147.html#a11493
204

hope that helps,

Gabriel

-Justin

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

!DSPAM:4007,47419f7795487082231907!

--
Justin Deoliveira
The Open Planning Project
http://topp.openplans.org