Add options JAVA_OPTS and HEAP_SIZE to startup script.

[janhoy]

Fixes #124

[ksclarke]

Er, realizing I probably should ask this here instead of on the issue/ticket...

Thanks for the PR. Just to clarify, do you want to add the specific heap size in addition to the percentage? Or would you want to use the fixed heap size instead of the percentage? We moved from fixed size to percentage when we started running in Kubernetes, but I'm unclear if there is an environment in which a person would want both(?)

@cachemeoutside Also, weren't you talking about the need for JAVA_OPTS for something else recently (I'm forgetting what it was exactly)? If so, maybe we want to split out that option from the fixed heap setting?

[ksclarke]

Actually, looking at this again now that I'm a little more awake... this doesn't prevent us from using JAVA_OPTS independent from the fixed heap setting so it should be good to go, as is, I think. @cachemeoutside do you agree?

[ksclarke]

Okay, interesting, the automated build failed because of the missing ENVs. I thought GitHub was giving us the chance to review it before we ran the actions so that we could confirm the PR wasn't doing anything it shouldn't, but it seems the secrets are still not available to the PR even after reviewing a PR from a fork.

I'm learning a lot about GitHub forked PRs today. So I guess we have to test PRs from forks locally and then override the merge block with admin privs? That seems like a bit of a pain.

@cachemeoutside do you know another way around this? I guess we could just skip the kakadu part of the build on forked PRs.

[ksclarke]

@janhoy would you mind pulling in the last commit from the base/main branch? I thought I'd be able to do it (without creating my own branch), but it seems that because this is a remote fork I cannot. That latest commit will hopefully stop the automated build from failing on remote PRs.

[janhoy]

@janhoy would you mind pulling in the last commit from the base/main branch? I thought I'd be able to do it (without creating my own branch), but it seems that because this is a remote fork I cannot. That latest commit will hopefully stop the automated build from failing on remote PRs.

Done

[cachemeoutside]

Wouldn't -Xmx and -XX:MaxRAMPercentage conflict?

re: JAVA_OPTS I'm in support of this in general, but I'm not sure about defining HEAP_SIZE in the Dockerfile. The simpler we can keep our Dockerfile, the better. We already have -XX:MaxRAMPercentage for out of the box deployments.

I guess it just really depends how simple we want to parameterize the JVM settings. In our case, there were times I just ended up overriding(through k8s) the entrypoint to specify the JVM settings I needed. I don't have a strong opinion other than the fact this would add more logic into the Dockerfile that we'd have to keep track of.

[ksclarke]

My understanding is that -Xmx and -XX:MaxRAMPercentage have a complementary relationship... if -Xmx exists, it's used; if it doesn't exist, then -XX:MaxRAMPercentage is used. For our use case, we wouldn't set -Xmx and the percentage would be used, but this gives us the option to allow someone else to set the -Xmx if they want.

[ksclarke]

@janhoy Just to check back in, are you still thinking you'd prefer to use -Xmx over the percentage?

[janhoy]

@janhoy Just to check back in, are you still thinking you'd prefer to use -Xmx over the percentage?

That's what we have been using, but I'm willing to skip that if clause of the PR and just let it be a general purpose variable to pass in whatever a user needs. We may convert to specifying RAM in docker-compose instead.. I can push a change

[ksclarke]

That's a good point too. We could just keep the JAVA_OPTS in line 15 and people could pass in whatever they want there. Do you want to remove 9-12 and we'll just merge the JAVA_OPTS addition?

[janhoy]

Done. I also added the var to Dockerfile and a short documentation to README so people are aware of it.

[ksclarke]

Fantastic, thanks!