Conversation
poikilotherm
added
Type: Feature
|
Hey @poikilotherm - I'm not sure I have an answer, but what are your thoughts on how often we should upgrade Payara? I know we upgraded last release as well. It sounds like in this case it's unlocking some specific functionalities that you will unblock you, which is great. But, do we expect to continue this pattern of upgrading every release? |
|
I'm not sure about a good answer for that, @djbrooke . Currently, Payara Community Releases are roughly cutted every two to three months, which is quite similar to the frequency we do Dataverse releases. Recently, the addition of MicroProfile Config 2.0 to Payara is a huge thing. As long as there are no security updates with the next release of Payara or any other huge bug resolved, we might be safe to skip it. For installations using Ansible or containers, updating Payara is not realy a big deal. I understand that for a classic installation, that might be a problem. Maybe someone could create a script that does the update process more conveniently? It might manage at least parts of domain.xml, too, so updates get even easier when we want people to do changes to their domain config. I think we should also ask the community about their opinion and which way to go. Usually production use cases benefit from regular and tested updates not just for the application but also the environment. Any production install should have some sort of strategy or at least opinion on how things should be dealt with. We could also envision a second stable branch like |
|
Thanks @poikilotherm - it sounds like you've thought about this and have some examples/strategies from other community-supported projects, which is great. I appreciate it. Like I mentioned, I'll rely on more technical people to provide some other opinions, but it was just something I wanted to flag for further discussion. |
pdurbin
left a comment
pdurbin
left a comment
There was a problem hiding this comment.
Overall, this looks great. I just have a question about the release notes.
| Some changes in this release require an upgrade to Payara 5.2021.1 or higher. | ||
|
|
||
| Instructions on how to update can be found in the | ||
| [Payara documentation](https://docs.payara.fish/community/docs/5.2021.1/documentation/user-guides/upgrade-payara.html) No newline at end of file |
There was a problem hiding this comment.
In the 5.3 release notes ( https://github.com/IQSS/dataverse/releases/tag/v5.3 ) we said:
It would likely be safer to upgrade Payara first, while still running Dataverse 5.2, and then proceed with the steps below. Upgrading from an earlier version of Payara should be a straightforward process: Undeploy Dataverse; stop Payara; move the current Payara directory out of the way; unzip the new Payara version in its place; replace the brand new payara/glassfish/domains/domain1 with your old, preserved domain1; start Payara, deploy Dataverse 5.2. We still recommend that you read the detailed upgrade instructions above; and, if you run into any issues with this upgrade, it will help to be able to separate them from any problems with the upgrade of Dataverse proper.
@poikilotherm how do you feel about putting equivalent text here? We can always remove it, especially at release time, if we decide we don't need it.
There was a problem hiding this comment.
Ok. If we merge the other PR containing changes only compatible with an upgraded Payara, we should mention updating the environment first before deploying the new WAR.
Do you feel like pushing a maintainer commit?
There was a problem hiding this comment.
@djbrooke mention this, but I am wary of upgrading Payara too often. I get that we took way too long to upgrade from Glassfish 4, but we don't want every release to include a Payara upgrade. (we should try to limit them to some more standard schedule - once a year, maybe 1 every 6 months). Unless of course there is a critical security fix.
There was a problem hiding this comment.
@poikilotherm sure, done in 4e94a80
@scolapasta sure, but I saw "Unblocks #7245, #7695 and maybe #7680" and figured it's probably worth it. I'll put it in QA but please feel free to pull it out.
There was a problem hiding this comment.
Right, but one is a draft, one is not yet worked on, and the third we have a solution for that does not require an app server upgrade. So I don't really want this to get merged, at least not for 5.4 and then to decide when we want to have our next infrastructure upgrade like this.
Don't get me wrong, I want us to upgrade Payara, I just want us to manage it well, as it does require overhead for developers, for reviewers, for QA.
Let's have a tech hours discussion this week or next on it.
|
I just noticed some Swagger/OpenAPI goodness is coming in Payara 5.2021.2 according to #5794 (comment) so maybe we should upgrade to .2 instead of .1. |
Already in progress. |
poikilotherm
force-pushed
the
7700-payara-5.2021.1
branch
from
e799f48 to
804086e
Compare
|
Whoops my bad. Thx @djbrooke |
|
@poikilotherm I'm running into an issue deploying, gives the error: Internal Exception: java.sql.SQLException: Error in allocating a connection. Cause: Access denied to execute this method : setURL I recall seeing this the last time we upgraded payara but not sure what the answer was then. FYI, I have not upgraded postgres to 13 yet due to syncing with Harvard's installation and it being only recommended. Is this a db driver version issue? Is it maybe something to do with the postgres driver being part of the war file now and perhaps a version issue? |
|
Hi @kcondon 🙂 I am very confident this has nothing to do with an upgrade of Payara itself, as I have been deploying recent develop branch inside containers all day long 🙈 (yesterday). (So I don't have the latest merges tested you made yesterday...) This makes me also confident it's not related to the driver included in the WAR or its version. It might be Postgres 9.6 vs 13, but I did not test that (using 13 inside container). Many deployment issues seem to arise from "dirty" appserver environments (leftovers, ...), which is pretty normal for devs not deploying inside a fresh container everytime. There is a chance that the domain.xml has necessary changes in the newer version that would need to be applied to a copy of an old domain. As I am always deploying to a clean env, I can't catch those problems. I could create a diff between the old and new version, if you want. Could you include a more verbose error log file here and/or maybe test with a fresh Payara install? |
|
Hi @poikilotherm I was following the release notes upgrade instructions as folks with prior installations won't have a clean environment. I understand about not being able to consider all cases but this is an important one I think. Here is the server.log error and tbh you and I saw this error in the past. There are two traces, the first has my db name listed twice (dvndbsimpledvndbsimple), that was also a prior issue we both had seen. Perhaps that's the root cause? [2021-06-29T18:35:49.038+0000] [Payara 5.2021.4] [SEVERE] [] [javax.enterprise.resource.resourceadapter.com.sun.gjc.util] [tid: _ThreadID=134 _Thr [2021-06-29T18:35:49.039+0000] [Payara 5.2021.4] [SEVERE] [] [javax.enterprise.resource.resourceadapter.com.sun.gjc.util] [tid: _ThreadID=134 _Thr java.lang.reflect.InvocationTargetException [2021-06-29T18:35:49.039+0000] [Payara 5.2021.4] [WARNING] [] [javax.enterprise.resource.resourceadapter.com.sun.enterprise.resource.allocator] [tid: _ThreadID=134 _ThreadName=payara-executor-service-scheduled-task] [timeMillis: 1624991749039] [levelValue: 900] [[ [2021-06-29T18:35:49.040+0000] [Payara 5.2021.4] [WARNING] [] [javax.enterprise.resource.resourceadapter.com.sun.enterprise.connectors] [tid: _ThreadID=134 _ThreadName=payara-executor-service-scheduled-task] [timeMillis: 1624991749040] [levelValue: 900] [[ [2021-06-29T18:35:49.040+0000] [Payara 5.2021.4] [WARNING] [] [javax.enterprise.resource.resourceadapter.com.sun.gjc.spi] [tid: _ThreadID=134 _ThreadName=payara-executor-service-scheduled-task] [timeMillis: 1624991749040] [levelValue: 900] [[ [2021-06-29T18:35:49.040+0000] [Payara 5.2021.4] [SEVERE] [] [org.eclipse.persistence.session./file:/usr/local/payara5/glassfish/domains/domain1/applications/dataverse-5.5/WEB-INF/classes/_VDCNet-ejbPU.ejb] [tid: _ThreadID=134 _ThreadName=payara-executor-service-scheduled-task] [timeMillis: 1624991749040] [levelValue: 1000] [[ Local Exception Stack: |
|
Sorry @kcondon I really can't recall this issue. Looked around in #7419 and in issues, but could not find that. I'm really wondering how the URL could be set to sth like "dvndbsimpledvndbsimple" as it should contain more than just that looking at http://github.com/IQSS/dataverse/blob/4c7d0afd7a9a643e4e79888e5a0feae9275f400f/src/main/java/edu/harvard/iq/dataverse/util/DataSourceProducer.java#L21-L21 You are sure that this env is clean? This looks completely random to me. 😞 |
|
Holy moly, was this from payara/Payara#4864, which was why we waited for 5.2020.6? Yes!!! #7048 (comment) |
|
@poikilotherm The previously encountered errors were ones we had discussed personally, I was looking for a record but cannot find them yet, unfortunately. The environment is a "real world" environment -my build box actually. It is a legitimate test and a necessary upgrade at any rate. I will also test a fresh install but this is the one I'm dealing with at the moment, the problem I'm reporting. |
|
There is what you did last time 😄 #7048 (comment) But these vars should be around now, right? |
|
Yes!!! Thanks for finding it. I'd thought it had something to do with the driver (pr anyway) ;) |
|
BTW: my deepest respect for your good memories with this one! 🙏 I completely forgot about it. |
|
As we are upgrading to a very recent version, we can now include actually working defaults in the data source definition. We could use a sane hostname like "postgresql" for the hostname (the default from the container images) and "5432" for the port. If "postgresql" could not be resolved as a hostname, the error message should be much more understandable. BTW you should have some log messages about missing MPCONFIG values. I added this to Payara 5.2021.1 IIRC |
|
I do not have those vars in my domain.xml, not sure why. I'm also running on .6 payara. I suppose I'd done a clean install at some point? Regardless, adding host and port vars corrected the problem. So in terms of upgrade, can we assume those vars will be in place? Also, why does it work without those with develop branch? 5.3 release notes say: (If you are using a PostgreSQL server on localhost:5432, you can omit dataverse.db.host and dataverse.db.port.) . That is my config so I shouldn't need them but my note in the driver pr says I needed them anyway. Not sure how this discrepancy remained unresolved, unless it is an initial deployment issue only? |
|
clears throat I have more intel about the correct timings now. I learned that vars living inside 🥺 It might be a good idea to correct the 5.3 release notes... |
|
@pdurbin says on Matrix we should focus on Payara in this PR. |
|
Yeah, I figure we'll have a chance to revisit stuff beyond the Payara upgrade in pull request #7695 (which depends on the upgrade). |
|
OK, so assuming we fix the v5.3 notes, we can then count on host and port to be there after an upgrade? Should be mention them in passing in these notes as a reminder: make sure you followed the v5.3 db conn update if upgrading from earlier version, particularly, user, password, host, port? or is that overkill? Meanwhile I've smoke tested and all is good. Will now do a clean install and perhaps merge unless there is some further clarification in docs/notes needed? Opinions? |
|
I'd be fine with using "localhost" as a default for the server part. Should serve lots of people, devs & admins alike, good things. It's easy to switch for me in the container images by setting an default env var. |
|
@poikilotherm Not sure what you mean -are they needed in domain.xml because there are no defaults? In that case we'd need both localhost and 5432, just tested ;) I need to leave soon to be somewhere but will do a quick installation test. After that, assuming it works we could merge unless there are other changes being discussed. And since I'm here, this is still broken |
|
Yes, there are no defaults in place as is. (Regarding Solr: I wasn't the one who broke that. There is WIP in #7904) |
|
OK, the new install worked fine, do we merge or make changes? |
6 participants
What this PR does / why we need it:
Update to Payara 5.2021.4. New features like MPCONFIG 2.0 ahead.
Which issue(s) this PR closes:
Closes #7700
Unblocks #7245, #7695 and maybe #7680, might be beneficial for #5794
Special notes for your reviewer:
Did I miss any place after grepping for "5.2020.6"?
Suggestions on how to test this:
Install Payara 5.2021.1, compile, package, deploy. Using regular integration tests, don't we?
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Nope.
Is there a release notes update needed for this change?:
Yes. 🔋 included.
Additional documentation:
None