Skip to content

GEOS-11607 Bugfix - KML WMS GetMap is performing a heavy database load query #8035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 9, 2024

Conversation

tiago-s-vieira-alb
Copy link
Contributor

@tiago-s-vieira-alb tiago-s-vieira-alb commented Nov 18, 2024

GEOS-11607 Powered by Pull Request Badge

JIRA ISSUE:
https://osgeo-org.atlassian.net/browse/GEOS-11607

Checklist

For core and extension modules:

  • New unit tests have been added covering the changes.
  • Documentation has been updated (if change is visible to end users).
  • The REST API docs have been updated (when changing configuration objects or the REST controllers).
  • There is an issue in the GeoServer Jira (except for changes that do not affect administrators or end users in any way).
  • Commit message(s) must be in the form [GEOS-XYZWV] Title of the Jira ticket.
  • Bug fixes and small new features are presented as a single commit.
  • Each commit has a single objective (if there are multiple commits, each has a separate JIRA ticket describing its goal).

@tiago-s-vieira-alb
Copy link
Contributor Author

Tests are failing. We'll take a look.

@tiago-s-vieira-alb
Copy link
Contributor Author

@aaime , the KMLTest "testRasterLayer" is failing after our change, on this assertion:

assertXpathEvaluatesTo("1", "count(//kml:GroundOverlay)", dom);

[ERROR] testRasterLayer(org.geoserver.kml.KMLTest) Time elapsed: 0.021 s <<< FAILURE! junit.framework.ComparisonFailure: expected:<[1]> but was:<[0]> at org.geoserver.kml.KMLTest.testRasterLayer(KMLTest.java:772)

I'm not familiar with the GroundOverlay attribute, and if it makes sense the changed result value.

Can you take a look please?

Thanks

@jratike80
Copy link
Contributor

I am bad at reading any code, but I try.

By https://github.com/geoserver/geoserver/pull/8035/files
the bbox is &bbox=-1.5,2,1.5,4&srs=EPSG:4326");
an then
assertXpathEvaluatesTo( "4.00243024094668", "//kml:Folder/kml:LookAt/kml:latitude", doc);
that does not fit within the bbox.

Maybe because of this the count is 0 instead of the expected 1? I do not know why "4.00243024094668" is expected to be included, but perhaps it is due to the Earth curvature.

Sorry if my suggestions do not make sense.

@tiago-s-vieira-alb
Copy link
Contributor Author

tiago-s-vieira-alb commented Nov 18, 2024

The unit test that I've done testLayerLookAt executes successfully.
The problem is with an existing unit test testRasterLayer that started to crash, when changing the decorator for the Layer.

Btw,
I’ve done a unit test using the “Basic polygons” shapefile that already exists in the project.
The bbox that i used was the blue rectangle, in order to grab two squares, and have a bigger final bbox.
2024-11-18 01 30 27

@aaime
Copy link
Member

aaime commented Nov 25, 2024

@tiago-s-vieira-alb when running against a raster layer context.getCurrentFeatureCollection returns null, a NPE occurs, and the test fails.

Something like this should address the issue (untested):

            Layer currentLayer = context.getCurrentLayer();
            Envelope bounds;
            if (currentLayer instanceof FeatureLayer) {
                bounds = context.getCurrentFeatureCollection().getBounds();
            } else {
                bounds = currentLayer.getBounds();
            }

@tiago-s-vieira-alb
Copy link
Contributor Author

It works!
Thank you

@tiago-s-vieira-alb
Copy link
Contributor Author

tiago-s-vieira-alb commented Dec 2, 2024

Hi,

The pipeline "Linux JDK GitHub CI / build (21, temurin, ubuntu-22.04)" is failing because:

Error: 6,291 [ERROR] Could not acquire lock(s)
Error: 6,291 [ERROR] java.lang.IllegalStateException: Could not acquire lock(s)
Error: 6,291 [ERROR] Could not acquire lock(s)
Error: 6,291 [ERROR] Could not acquire lock(s)
Error: 6,291 [ERROR]

How can I launch it again?
It doesn't seem to be related with the changes of this PR.

@mprins
Copy link
Member

mprins commented Dec 3, 2024

java.lang.IllegalStateException: Could not acquire lock(s)

that problem seems to be fixed in the main branch (as of #8063), if you rebase your branch it should pass.

for now I've restarted the failing build

@tiago-s-vieira-alb
Copy link
Contributor Author

Same error, but now in the "Assembly GitHub CI / build (ubuntu-22.04, 11, temurin)"

@tiago-s-vieira-alb
Copy link
Contributor Author

All green! :)
May you proceed with the merge to main?
Thanks

@aaime aaime merged commit 26ed595 into geoserver:main Dec 9, 2024
16 checks passed
@aaime aaime added backport 2.25.x Instructs the bot to create a 2.25.x backport PR on merge backport 2.26.x Instructs the bot to create a 2.26.x backport PR on merge labels Dec 9, 2024
@geoserver-bot
Copy link
Collaborator

The backport to 2.25.x failed:

The process '/usr/bin/git' failed with exit code 128
stderr
error: commit 2881042e6616b4d45b0f366da2be58010708b3a3 is a merge but no -m option was given.
fatal: cherry-pick failed

stdout
Auto-merging src/kml/src/test/java/org/geoserver/kml/KMLTest.java
[backport-8035-to-2.25.x a4a6448469] GEOS-11607 Bugfix - KML WMS GetMap is performing a heavy database load query
 Author: Tiago <[email protected]>
 Date: Mon Nov 18 02:06:03 2024 +0000
 2 files changed, 20 insertions(+), 1 deletion(-)
[backport-8035-to-2.25.x dad973cbd2] GEOS-11607 Bugfix - KML WMS GetMap is performing a heavy database load query
 Author: Tiago <[email protected]>
 Date: Mon Nov 25 14:46:04 2024 +0000
 1 file changed, 9 insertions(+), 1 deletion(-)
Auto-merging src/kml/src/test/java/org/geoserver/kml/KMLTest.java
[backport-8035-to-2.25.x 3070324057] GEOS-11607 Bugfix - KML WMS GetMap is performing a heavy database load query
 Author: Tiago <[email protected]>
 Date: Mon Nov 25 17:06:06 2024 +0000
 1 file changed, 2 insertions(+), 4 deletions(-)

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.25.x 2.25.x
# Navigate to the new working tree
cd .worktrees/backport-2.25.x
# Create a new branch
git switch --create backport-8035-to-2.25.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick 624d243860d5ecd3d234b3f225a7e615a4b417a2,958b11a0082ada209c81fe6a5eab17f0e7a0d487,f13c63b022b2725e10826b53d10e25963cab30a1,2881042e6616b4d45b0f366da2be58010708b3a3
# Push it to GitHub
git push --set-upstream origin backport-8035-to-2.25.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.25.x

Then, create a pull request where the base branch is 2.25.x and the compare/head branch is backport-8035-to-2.25.x.

@geoserver-bot
Copy link
Collaborator

The backport to 2.26.x failed:

The process '/usr/bin/git' failed with exit code 128
stderr
error: commit 2881042e6616b4d45b0f366da2be58010708b3a3 is a merge but no -m option was given.
fatal: cherry-pick failed

stdout
[backport-8035-to-2.26.x 5214b243b9] GEOS-11607 Bugfix - KML WMS GetMap is performing a heavy database load query
 Author: Tiago <[email protected]>
 Date: Mon Nov 18 02:06:03 2024 +0000
 2 files changed, 20 insertions(+), 1 deletion(-)
[backport-8035-to-2.26.x 2ba582cfc7] GEOS-11607 Bugfix - KML WMS GetMap is performing a heavy database load query
 Author: Tiago <[email protected]>
 Date: Mon Nov 25 14:46:04 2024 +0000
 1 file changed, 9 insertions(+), 1 deletion(-)
[backport-8035-to-2.26.x aba85eae41] GEOS-11607 Bugfix - KML WMS GetMap is performing a heavy database load query
 Author: Tiago <[email protected]>
 Date: Mon Nov 25 17:06:06 2024 +0000
 1 file changed, 2 insertions(+), 4 deletions(-)

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.26.x 2.26.x
# Navigate to the new working tree
cd .worktrees/backport-2.26.x
# Create a new branch
git switch --create backport-8035-to-2.26.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick 624d243860d5ecd3d234b3f225a7e615a4b417a2,958b11a0082ada209c81fe6a5eab17f0e7a0d487,f13c63b022b2725e10826b53d10e25963cab30a1,2881042e6616b4d45b0f366da2be58010708b3a3
# Push it to GitHub
git push --set-upstream origin backport-8035-to-2.26.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.26.x

Then, create a pull request where the base branch is 2.26.x and the compare/head branch is backport-8035-to-2.26.x.

@aaime
Copy link
Member

aaime commented Dec 9, 2024

Merged.
The bot failed to backport despite my squash-merge, if you want to have it on 2.26.x and 2.25.x you'll have to cherry-pick 26ed595 manually and issue PRs targeting the respective branches.

@tiago-s-vieira-alb
Copy link
Contributor Author

Merged. The bot failed to backport despite my squash-merge, if you want to have it on 2.26.x and 2.25.x you'll have to cherry-pick 26ed595 manually and issue PRs targeting the respective branches.

We can wait for the main release (27.x).
Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.25.x Instructs the bot to create a 2.25.x backport PR on merge backport 2.26.x Instructs the bot to create a 2.26.x backport PR on merge failed backport
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants