Skip to content
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

Microprofile recursive update exception when trying to use the Alias/DBSetting sources #7680

Closed
qqmyers opened this issue Mar 12, 2021 · 3 comments · Fixed by #8823
Closed
Milestone

Comments

@qqmyers
Copy link
Member

qqmyers commented Mar 12, 2021

In #7661, I tried to use the new Alias and DBSetting ConfigSources to support using a normal db setting aliased to an underlying microprofile variable for two cases. One appears to work fine, but the other was consistently giving me a recursive update exception. The problem occurred in #7661 prior to this commit, which includes the (hopefully) relevant part of the stack trace I was seeing in the commit notes.

At this point, I don't know if the problem was due to how I was using it, or if the problem is general- the only difference I can see between the setting that worked and the one that didn't is that the method retrieving the :InstallationName setting/associated mp variable is used in more places/possibly in async and regular code, but that could be a red-herring.

From the stack trace, it looked like the issue was related to the Alias source getting called recursively (it's getValue method calls ConfigProvider.getConfig.getOptionalValue() when you request the new variable (that is aliased to an old variable name served by the DbSettings source)). That could be a design issue (as far as I can tell, #7661 is the first use of this code outside of tests) so I though it was worth an issue. If it just innapropriate use, it might be worth some guidance in the dev docs for microprofile stuff.

At this point, the issue doesn't affect any code that is/will be merged, but it will probably appear again if we try to update other settings. I think #7661 prior to the commit above is ~working code (there are some changes after that commit needed to make everything work, but as of that commit Dataverse runs and nothing is broken except this issue, which can be triggered by publishing or reExporting via api), and I'm happy to answer questions if anything is unclear.

@poikilotherm
Copy link
Contributor

poikilotherm commented Mar 15, 2021

@qqmyers thanks for opening the issue.
Gave this a shot today by trying to add Arquillian based testing to make this better testable and reproducable in a real CDI environment.
Stumbled over Tika including CXF breaking Arquillian and found #7684 along the way. Will proceed with this tomorrow.

@poikilotherm
Copy link
Contributor

As outlined in #7639, I will remove the DBSettingConfigSource, so this potential issue should vanish.
Leaving this open for now (to be closed with a PR for #7639), yet not doing any more investigation for now.

poikilotherm added a commit to poikilotherm/dataverse that referenced this issue Jun 9, 2021
Thanks to @qqmyers, we found out that the config source reading
from the database is having multiple issues. See IQSS#7680 and IQSS#7639
for details.
poikilotherm added a commit to poikilotherm/dataverse that referenced this issue Jun 20, 2022
It doesn't seem we are going to use MPCONFIG as the
primary source of settings from the database for now.
This commit removes the DbConfigSource which has had
errors, too.

Fixes IQSS#7680
poikilotherm added a commit to poikilotherm/dataverse that referenced this issue Jun 30, 2022
It doesn't seem we are going to use MPCONFIG as the
primary source of settings from the database for now.
This commit removes the DbConfigSource which has had
errors, too.

Fixes IQSS#7680
@pdurbin
Copy link
Member

pdurbin commented Sep 19, 2022

This issue was closed automatically when I merged #8823 so I'm giving it the milestone for the next release. As I noted at #8823 (comment) upgrading Payara fixed it for me.

@pdurbin pdurbin added this to the 5.12 milestone Sep 19, 2022
poikilotherm added a commit to poikilotherm/dataverse that referenced this issue Sep 19, 2022
It doesn't seem we are going to use MPCONFIG as the
primary source of settings from the database for now.
This commit removes the DbConfigSource which has had
errors, too.

Fixes IQSS#7680
poikilotherm added a commit to poikilotherm/dataverse that referenced this issue Sep 20, 2022
It doesn't seem we are going to use MPCONFIG as the
primary source of settings from the database for now.
This commit removes the DbConfigSource which has had
errors, too.

Fixes IQSS#7680
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants