NIFI-5186: Updating UI to support asynchronous validation #2722
Conversation
- Updating UI to support showing when a component is validating.
|
@mcgilman @markap14 this test finding is related to Mark's preceding PR for this most likely but `[ERROR] Tests run: 15, Failures: 1, Errors: 0, Skipped: 2, Time elapsed: 16.555 s <<< FAILURE! - in org.apache.nifi.controller.scheduling.TestProcessorLifecycle ` Otherwise, I'm doing some functional testing now and obviously contrib-check is good. This is really cool and will greatly aid flow performance for large scale flows or user code in validation that doesn't behave nicely. |
|
@markap14 @mcgilman in looking at this from a user point of view a couple thoughts: When validating things which are slow and things which are fast it appears fast things can be bottlenecked behind fast things. This makes sense but can we do better? For instance I have a DebugFlow with a 10 sec validation cycle and an UpdateAttribute processor. it took 10 secs or more for UpdateAttribute to come back as valid presumably because it was waiting for the next validation cycle and then perhaps it had to wait behind the slow DebugFlow proc. |
|
@joewitt your observations are very interesting - i am not seeing that at all. I created a DebugFlow with a validation pause of 10 seconds. Then I created an update attribute. UpdateAttribute was immediately invalid. I triggering validation to occur on DebugFlow and then UpdateAttribute and saw instance responses for UpdateAttribute's validation. So to answer your questions specifically:
Of note, when you get back the "Validating" icon, that icon will not go away until the status is refreshed, which happens by default every 30 seconds. If you right-click on the canvas and refresh, that will also refresh it. Just wanted to make sure that you did refresh the canvas in that time interval? |
|
@joewitt re: the unit test failure, I don't believe that it's actually related to the previous PR necessarily, but is just a timing issue that happened to trigger here. We could certainly raise the timeout used - it was waiting up to 2 seconds for the processor state to become STOPPED in the unit test. We could certainly raise that to 10 seconds or so. I think the timeout chosen was fairly arbitrary and increasing it would only increase the amount of time that the tests takes to complete when needed to. If the state transitions to STOPPED after 25 milliseconds then that's all that it'll wait. |
| // can be driven by environmental conditions. this check allows any of those to | ||
| // take precedence over the configured run status. | ||
| if (RunStatus.Invalid.name().equals(toMerge.getRunStatus())) { | ||
| target.setRunStatus(RunStatus.Invalid.name()); | ||
| if (RunStatus.Validating.toString().equals(toMerge.getRunStatus())) { |
There was a problem hiding this comment.
It's probably best to use .name() instead of .toString(), as toString() could potentially change at any time, whereas the name never will
There was a problem hiding this comment.
I was just addressing an inconsistency because in the DtoFactory we populate run status with the value from toString().
|
@joewitt I think I understand now what you were seeing. If I create two different DebugFlow processors, each with a validation pause of 10 seconds, and then I create an UpdateAttribute and configure each of them in quick succession, I see UpdateAttribute in a 'Validating' state for many seconds. I have created a new JIRA for this, though, NIFI-5222, as I don't think it's something that should really block merging this PR in. The issue appears to be that we kick off validation 3 times each time that we click "Update" and so if we update both DebugFlow processors, we end up kicking off 6 validation tasks instead of 2, and that ends up blocking UpdateAttribute since we have only 5 threads in the pool. |
| var bState = 'Invalid'; | ||
| if (nfCommon.isEmpty(b.component.validationErrors)) { | ||
| var bState; | ||
| if (a.component.validationStatus === 'VALIDATING') { |
There was a problem hiding this comment.
should this be checking b.component.validationStatus?
| if (nfCommon.isEmpty(b.component.validationErrors)) { | ||
| var bState; | ||
| if (a.component.validationStatus === 'VALIDATING') { | ||
| bState = 'Validating'; |
There was a problem hiding this comment.
should this be checking b.component.validationStatus
- Addressing sort issue discovered during PR.
|
Thanks @mcgilman this has been merged to master. |
NIFI-5186: