[Geoserver-devel] [jira] Created: (GEOS-632) Don't ask for FID's in WMS requests

Don't ask for FID's in WMS requests
-----------------------------------

         Key: GEOS-632
         URL: http://jira.codehaus.org/browse/GEOS-632
     Project: GeoServer
        Type: Improvement

    Versions: 1.3.0
    Reporter: Chris Holmes
Assigned to: dblasby
     Fix For: 1.3.x

Dave made some nice code improvements to allow datastores to not return FIDs, which renderers obviousl don't really need. This gives a bit of a performance improvement.

I eliminted the code that does it, since it was messing things up within the renderer. But I think we can set the hint in _GeoServer_, by making a new transaction for the WMS requests, and closing that transaction when done.

I'm not sure how easy it is to do, if we can set a transaction on the feature source before sending to the renderer. But if we can, this is the code to do it:

Transaction t= new DefaultTransaction();
             t.putProperty("doNotGetFIDS",Boolean.TRUE);
                    fs.setTransaction(t);

Then after the renderer does its thing, call commit on the transaction, so that the connection gets returned to its pool.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://jira.codehaus.org/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira

Dont do this in the feature source.

You really need to do this in the renderer. Imagine a Style that has
a FID Filter in it - for example a simple highlight-a-feature web app.
This can get a bit more complex if the FID filter is in Rules with
Min/Max ScaleDenominators. This is all (optimally) handled by the
implementation in the renderer; its probably inappropriate to put
this type of logic in the DefaultRasterMapGenerator geoserver class.

It sounds like the renderer isnt finishing up transactions when its
done drawing a layer (ie. your other bug report - 631 I believe).
They should, theoretically, be fixed by adding a Finally clause to the
render-a-layer-loop that will also closes the transaction (it
currently only closes the reader). It appears the renderer assumes
that the transaction is auto-commit, so it should be fairly easy to
clean this up, perhaps by just adding a single line of code like
#getTransaction().commitTransaction().

Alternately, you could do the transaction committing
DefaultRasterMapGenerator (and EVERY other class that communicates
with the renderer).

I think this just needs some documentation so that its properly
handled by either the renderer or the class-that-calls-the-renderer.
This should be discussed on the geotools list since it will affect
other project, like udig, as well.

There's arguments for either of these positions - the former (renderer
commit) makes things easier, but the latter could allow you to do
things like add features to a FeatureType, renderer it, then rollback
the additions.

I believe there's already a renderer hint to turn the "dont get FID"
feature on/off. If not, its trivial to add it (see any of the other
options).

dave
ps. watch out for a layer being rendered twice in a single paint().

On 5/31/06, Chris Holmes (JIRA) <jira@anonymised.com> wrote:

Don't ask for FID's in WMS requests
-----------------------------------

         Key: GEOS-632
         URL: http://jira.codehaus.org/browse/GEOS-632
     Project: GeoServer
        Type: Improvement

    Versions: 1.3.0
    Reporter: Chris Holmes
Assigned to: dblasby
     Fix For: 1.3.x

Dave made some nice code improvements to allow datastores to not return FIDs, which renderers obviousl don't really need. This gives a bit of a performance improvement.

I eliminted the code that does it, since it was messing things up within the renderer. But I think we can set the hint in _GeoServer_, by making a new transaction for the WMS requests, and closing that transaction when done.

I'm not sure how easy it is to do, if we can set a transaction on the feature source before sending to the renderer. But if we can, this is the code to do it:

Transaction t= new DefaultTransaction();
                          t.putProperty("doNotGetFIDS",Boolean.TRUE);
                          fs.setTransaction(t);

Then after the renderer does its thing, call commit on the transaction, so that the connection gets returned to its pool.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://jira.codehaus.org/secure/Administrators.jspa
-
For more information on JIRA, see:
   Jira | Issue & Project Tracking Software | Atlassian

-------------------------------------------------------
All the advantages of Linux Managed Hosting--Without the Cost and Risk!
Fully trained technicians. The highest number of Red Hat certifications in
the hosting industry. Fanatical Support. Click to learn more
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=107521&bid=248729&dat=121642
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

David Blasby wrote:

Dont do this in the feature source.

You really need to do this in the renderer. Imagine a Style that has
a FID Filter in it - for example a simple highlight-a-feature web app.
This can get a bit more complex if the FID filter is in Rules with
Min/Max ScaleDenominators. This is all (optimally) handled by the
implementation in the renderer; its probably inappropriate to put
this type of logic in the DefaultRasterMapGenerator geoserver class.

It sounds like the renderer isnt finishing up transactions when its
done drawing a layer (ie. your other bug report - 631 I believe).
They should, theoretically, be fixed by adding a Finally clause to the
render-a-layer-loop that will also closes the transaction (it
currently only closes the reader). It appears the renderer assumes
that the transaction is auto-commit, so it should be fairly easy to
clean this up, perhaps by just adding a single line of code like
#getTransaction().commitTransaction().

Yeah, this gets tricky. Because the way the code that adds the FID hint in works is that it checks to make sure that it _is_ a transaction.AUTO_COMMIT. I think we talked about this Dave, we wanted to be nice in case someone _was_ trying to do something with a transaction in the renderer (though no one does currently).

So if we're to be safe in the same way, then we shouldn't close transactions in the renderer, since someone else may have passed in a transaction that they want to keep open...

Maybe we could put in another hint that says 'fids set in renderer', so then we'd be sure that the renderer was the one to make the transaction?

But yeah, I agree with your point Dave, I hadn't thought of that.

Alternately, you could do the transaction committing
DefaultRasterMapGenerator (and EVERY other class that communicates
with the renderer).

Yeah, that would be a bit unintuitive from an API perspective. Though it could work if it's a renderer option, that defaults to false, and in the javadocs we advise that when it's set to true that the client code must close the transaction.

I think this just needs some documentation so that its properly
handled by either the renderer or the class-that-calls-the-renderer.
This should be discussed on the geotools list since it will affect
other project, like udig, as well.

There's arguments for either of these positions - the former (renderer
commit) makes things easier, but the latter could allow you to do
things like add features to a FeatureType, renderer it, then rollback
the additions.

I believe there's already a renderer hint to turn the "dont get FID"
feature on/off. If not, its trivial to add it (see any of the other
options).

Cool, thoughts? Though I think the _right_ thing to do is to just put hints in the Query, since this is coming about because we're using transactions where they're not really needed. I don't think we should do that api change on 2.2.x, since we're in RC's. I'd say the best default is to have it be a rendering hint that is normally false, then GeoServer can set it to true and close the transaction. For now it's just turned off...

C

dave
ps. watch out for a layer being rendered twice in a single paint().

On 5/31/06, Chris Holmes (JIRA) <jira@anonymised.com> wrote:

Don't ask for FID's in WMS requests
-----------------------------------

         Key: GEOS-632
         URL: http://jira.codehaus.org/browse/GEOS-632
     Project: GeoServer
        Type: Improvement

    Versions: 1.3.0
    Reporter: Chris Holmes
Assigned to: dblasby
     Fix For: 1.3.x

Dave made some nice code improvements to allow datastores to not return FIDs, which renderers obviousl don't really need. This gives a bit of a performance improvement.

I eliminted the code that does it, since it was messing things up within the renderer. But I think we can set the hint in _GeoServer_, by making a new transaction for the WMS requests, and closing that transaction when done.

I'm not sure how easy it is to do, if we can set a transaction on the feature source before sending to the renderer. But if we can, this is the code to do it:

Transaction t= new DefaultTransaction();
                          t.putProperty("doNotGetFIDS",Boolean.TRUE);
                          fs.setTransaction(t);

Then after the renderer does its thing, call commit on the transaction, so that the connection gets returned to its pool.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://jira.codehaus.org/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira

-------------------------------------------------------
All the advantages of Linux Managed Hosting--Without the Cost and Risk!
Fully trained technicians. The highest number of Red Hat certifications in
the hosting industry. Fanatical Support. Click to learn more
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=107521&bid=248729&dat=121642
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

-------------------------------------------------------
All the advantages of Linux Managed Hosting--Without the Cost and Risk!
Fully trained technicians. The highest number of Red Hat certifications in
the hosting industry. Fanatical Support. Click to learn more
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=107521&bid=248729&dat=121642
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

--
Chris Holmes
The Open Planning Project
http://topp.openplans.org