ioos / erddap-gold-standard

Contains the 'gold standard' ERDDAP configuration, with datasets compliant with IOOS Metadata Profile 1.2
https://standards.sensors.ioos.us/erddap/index.html
8 stars 16 forks source link

use erddap 2.23 #48

Closed callumrollo closed 11 months ago

callumrollo commented 1 year ago

This partly resolves #47

MathewBiddle commented 12 months ago

I don't recall the differences between 2.18 and 2.23. If there are changes to the information in datasets.xml or other configurations, we should update those as well. See https://erddap.github.io/changes.html for changes between 2.18 and 2.23.

Some notable ones: https://erddap.github.io/changes.html#changes2.21

TO DO: For Java 17, you shouldn't use -d64 in JAVA_OPTS in setenv.bat or setenv.sh. So if it is there, please remove it. I think that 64 bit mode is now selected when you download a 64 bit version of Java. Thanks to Sam Woodman.

https://erddap.github.io/changes.html#changes2.19

TO DO: Recommended: If your server's CPU has 4+ cores and 8+ GB of RAM, consider changing to these settings in your datasets.xml file:

<nGridThreads>3</nGridThreads>
<nTableThreads>3</nTableThreads>

NEW: You can now put <updateMaxEvents>10</updateMaxEvents> in datasets.xml (in with the other settings near the top) to change the maximum number of file changes (default=10) that will be processed by the updateEveryNMillis system. A larger number (100?) may be useful when it is very important that the dataset be kept always up-to-date. See the updateMaxEvents documentation. Thanks to John Maurer.

There might be others too.

srstsavage commented 11 months ago

JAVA_OPT -d64 was removed from axiom/docker-erddap here:

https://github.com/axiom-data-science/docker-erddap/pull/49/files

and is in place for the last two release tags (axiom/docker-erddap:2.22-jdk17-openjdk and axiom/docker-erddap:2.23-jdk17-openjdk).

nGridThreads and nTableThreads are unset in datasets.xml so they should get good defaults depending on dynamic detection of available cores: https://github.com/ioos/erddap-gold-standard/blob/master/erddap/content/datasets.xml#L15-L16

srstsavage commented 11 months ago

@callumrollo GenerateDatasetsXml.sh also references a specific version of axiom/docker-erddap which needs to be updated to 2.23-jdk17-openjdk or latest

callumrollo commented 11 months ago

Thanks for the review @srstsavage . I believe the update to 2.23-jdk17-openjdk in GenerateDatasetsXml.sh in included in this PR. Are there more places wherer this change is required? I have left the files using 1docker-erddap:latest1 as they are (e.g. DasDds.sh)

@MathewBiddle datasets.xml currently defaults to to

<nGridThreads>3</nGridThreads>
<nTableThreads>3</nTableThreads>

with comments to set these values to 3 if the ERDDAP has more than 4 cores. Seems sensible.

As far as I can work out the other changes from 2.18 to 2.23 have been covered by axiom in the docker image itself

callumrollo commented 11 months ago

I think this is ready for merge @MathewBiddle unless there's more to add?

It would make to sense to merge it at the same time as #49 though to that the docs are aligned with the files

MathewBiddle commented 11 months ago

Looks great! Thanks for the effort! Merged #49 too.