[jira] [Comment Edited] (NIFI-12236) Improving fault tolerancy of the QuestDB backed metrics repository
[ https://issues.apache.org/jira/browse/NIFI-12236?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17794312#comment-17794312 ] Joe Witt edited comment on NIFI-12236 at 12/7/23 3:42 PM: -- Regarding defaults - thank you. API Changes: These should get the highest degree of scrutiny by reviewers and the most effort and thought by authors. At this stage I do not understand the intention for the change. It appears that concern is shared. If there is a non-API/specific to this implementation option that should be pursued instead. Suggestion about the nar: I am not aware of other cases where the optional implementation of a thing is the lighter option in terms of dependencies path. In this case the optional implementation (QuestDB backed stats) is the heavier one in terms of dependency/operations. By suggesting it be in its own nar I am not of the view it is a lot more work to be clear. I could be wrong. But the more work consideration is also not a strong one. The 'it is already in there...' argument is better since well - that is true. I'm just asking that the effort to do that be strongly considered because then those who would not use this option don't have to carry the component into deployment nor the potential vulnerabilities the libraries might bring. But those that want it are good to go and can use it. If it is baked into the framework nar this is not an option. As evidence of my concern regarding the vulnerability it does give pause that the version of QuestDB was not considered in this PR to this point. Keeping dependencies up to date is *everyones* responsibility. The NiFi 2.0 line is by the way super wildly close to being entirely free of static coding practice and dependency vulnerabilities (excluding hadoop related components) which is in itself a miracle. Please ensure the dependencies in this PR are up to date. On that note do we know what QuestDB version changes mean in terms of breaking changes, changes that might require users to do something to keep their state, etc..? I ask because we learned our lessons the hard way here with H2. H2 served us extremely well over the years but eventually it created some very complicated compelling events. Anyway - I dont want to come off like this thing can't happen. The root idea of this I think we all agree is useful. There is some debate on whether this implementation approach is desirable vs another but even that does not matter. He who does the work for a given implementation ultimately wins. What I am asking then though is please make that implementation as narrow and specific as possible to give others choice on whether they are exposed to it. That is the heart of the replies I am making. Thanks was (Author: joewitt): Regarding defaults - thank you. API Changes: These should get the highest degree of scrutiny by reviewers and the most effort and thought by authors. At this stage I do not understand the intention for the change. It appears that concern is shared. If there is a non-API/specific to this implementation option that should be pursued instead. Suggestion about the nar: I am not aware of other cases where the optional implementation of a thing is the light in terms of dependencies path. In this case the optional implementation (QuestDB backed stats) is the heavier one in terms of dependency/operations. By suggesting it be in its own nar I am not of the view it is a lot more work to be clear. I could be wrong. But the more work consideration is also not a strong one. The 'it is already in there...' argument is better since well - that is true. I'm just asking that the effort to do that be strongly considered because then those who would not use this option don't have to carry the component into deployment nor the potential vulnerabilities the libraries might bring. But those that want it are good to go and can use it. If it is baked into the framework nar this is not an option. As evidence of my concern regarding the vulnerability it does give pause that the version of QuestDB was not considered in this PR to this point. Keeping dependencies up to date is *everyones* responsibility. The NiFi 2.0 line is by the way super wildly close to being entirely free of static coding practice and dependency vulnerabilities (excluding hadoop related components) which is in itself a miracle. Please ensure the dependencies in this PR are up to date. On that note do we know what QuestDB version changes mean in terms of breaking changes, changes that might require users to do something to keep their state, etc..? I ask because we learned our lessons the hard way here with H2. H2 served us extremely well over the years but eventually it created some very complicated compelling events. Anyway - I dont want to come off like this thing can't happen. The root idea of this
[jira] [Comment Edited] (NIFI-12236) Improving fault tolerancy of the QuestDB backed metrics repository
[ https://issues.apache.org/jira/browse/NIFI-12236?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17794244#comment-17794244 ] Simon Bence edited comment on NIFI-12236 at 12/7/23 2:36 PM: - Hi [~pvillard]! Thanks for mentioning the admin guide! I forgot to add the newly wired out properties indeed. I will add those during the review. 1. I think we agreed on the defaulting, with review changes it will be changed back as well 2. As this looks to be more a nuanced question and I already have a discussion about it on the PR with David Hendermann, I suggest to continue this topic there. I am open for reverting those changes but before just doing that, I would like to be sure that the intentions for the change are clear. 3. I am not sure what Joe meant under moving it into a nar. If it is the already mentioned pluggability (which I totally in line with in the long run) I think it is not something to add to this PR, which is more like a glorified refactor story. If it is about something else, could you please specifiy it [~joewitt] ? (Note: the actual QuestDB related code is in a separate jar. All the code remained in the `nifi-framework-core` is related to the actual repository implementation (counterpart of the Volatile implementation was (Author: simonbence): Hei [~pvillard]! Thanks for mentioning the admin guide! I forgot to add the newly wired out properties indeed. I will add those during the review. 1. I think we agreed on the defaulting, with review changes it will be changed back as well 2. As this looks to be more a nuanced question and I already have a discussion about it on the PR with David Hendermann, I suggest to continue this topic there. I am open for reverting those changes but before just doing that, I would like to be sure that the intentions for the change are clear. 3. I am not sure what Joe meant under moving it into a nar. If it is the already mentioned pluggability (which I totally in line with in the long run) I think it is not something to add to this PR, which is more like a glorified refactor story. If it is about something else, could you please specifiy it [~joewitt] ? (Note: the actual QuestDB related code is in a separate jar. All the code remained in the `nifi-framework-core` is related to the actual repository implementation (counterpart of the Volatile implementation > Improving fault tolerancy of the QuestDB backed metrics repository > -- > > Key: NIFI-12236 > URL: https://issues.apache.org/jira/browse/NIFI-12236 > Project: Apache NiFi > Issue Type: Improvement > Components: Core Framework >Reporter: Simon Bence >Assignee: Simon Bence >Priority: Major > Time Spent: 1h 10m > Remaining Estimate: 0h > > Based on the related discussion on the dev email list, the QuestDB handling > of the metrics repository needs to be improved to have better fault tolerance > in order to be possible to use as a viable option for default metrics data > store. This should primarily focus on handling unexpeted database events like > corrupted database or loss of space on the disk. Any issues should be handled > with an attempt to keep the database service healthy but in case of that is > impossible, the priority is to keep NiFi and the core services running, even > with the price of metrics collection / presentation outage. -- This message was sent by Atlassian Jira (v8.20.10#820010)