[GitHub] nifi pull request: NIFI-1668 modified TestProcessorLifecycle to en...

[olegz]

Github user olegz commented on the pull request:

https://github.com/apache/nifi/pull/324#issuecomment-221935626
  
@markap14 the "idempotency" comment is valid and I agree for that alone we 
don't need multiple threads. I'll look again to see if test name matches with 
what I was actually trying to test there.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1668 modified TestProcessorLifecycle to en...

[markap14]

Github user markap14 commented on the pull request:

https://github.com/apache/nifi/pull/324#issuecomment-221935156
  
Agreed - I think moving the FlowController lifecycle to @Before and @After 
should address the issues. I do think that the 
validateIdempotencyOfProcessorStartOperation() test is a bit odd, though. Lots 
of tasks are kicked off in the background, which will test (to some degree) 
thread safety. However, if the goal is to test idempotency, then we should just 
call the method in the foreground multiple successive times, not run it in the 
background.

If the goal is to test running it in the background and the associated 
thread-safety, then I'd recommend that rather than using the countdown latch we 
just do something like:

```
while (testProcessor.operationNames.isEmpty() || 
testProcNode.getScheduledState() != ScheduledState.RUNNING) {
Thread.sleep(10L);
}
```

And then set a 10-second timeout on the test: @Test(timeout=1)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1668 modified TestProcessorLifecycle to en...

[olegz]

Github user olegz commented on the pull request:

https://github.com/apache/nifi/pull/324#issuecomment-221925828
  
"...how come the that issue wouldn't cause all following tests to fail..." 
some times it does. You may want to consult with Mark on that since that error 
originates from FC. Basically I think FC was designed to be singleton within 
VM, so when you have multiple instances (as in tests), things may happen.

Actually as you can see _coundDownCounter_ is decremented inside the task, 
so until the task is executed (some time in the future), the counter is not 
decremented. So this pattern essentially guarantees that when counter is down 
to 0, all threads have finished. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1668 modified TestProcessorLifecycle to en...

[olegz]

Github user olegz commented on the pull request:

https://github.com/apache/nifi/pull/324#issuecomment-221917899
  
The issue is not with before, but with _after_, since it effectively acts 
as a _finally_ block. The way it was before if a test failed for whatever 
reason, then the FC.shutdown was not executed, and due to that you had this 
file lock exception.

As for multiple-threading test, it is not black and white, but rather 
allows the system to validate if there are concurrency issues of some type. I 
am also not sure what you mean by scheduling issues as the test controls the 
thread pool. So may be you can clarify.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1668 modified TestProcessorLifecycle to en...

[JPercivall]

Github user JPercivall commented on the pull request:

https://github.com/apache/nifi/pull/324#issuecomment-221903401
  
Also I might be alone here, but I dislike the unit tests that create an 
executor to run something many times sometime in the future and then waiting to 
see if it will run/break. It is too prone to unit tests failing due to 
scheduling issues rather than the actual thing you are trying to test.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1668 modified TestProcessorLifecycle to en...

[JPercivall]

Github user JPercivall commented on the pull request:

https://github.com/apache/nifi/pull/324#issuecomment-221901178
  
How will moving the building of the flow controller to the before method 
help the OverlappingFileLockException? The before and after methods are run 
immediately before and immediately after each test, so they will get executed 
at almost the same time as before this change. The test is failing when it's 
creating the state provider, which is not being used. Why not just mock it so 
that the state provider doesn't attempt to grab a lock on the File System? 

This PR should also address the 
validateIdempotencyOfProcessorStartOperation method which fails many times too 
but in a different fashion:

Tests run: 16, Failures: 1, Errors: 1, Skipped: 0, Time elapsed: 40.952 sec 
<<< FAILURE! - in org.apache.nifi.controller.scheduling.TestProcessorLifecycle

validateIdempotencyOfProcessorStartOperation(org.apache.nifi.controller.scheduling.TestProcessorLifecycle)
  Time elapsed: 0.162 sec  <<< FAILURE!
java.lang.AssertionError: expected:<1> but was:<0>
at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.failNotEquals(Assert.java:834)
at org.junit.Assert.assertEquals(Assert.java:645)
at org.junit.Assert.assertEquals(Assert.java:631)
at 
org.apache.nifi.controller.scheduling.TestProcessorLifecycle.validateIdempotencyOfProcessorStartOperation(TestProcessorLifecycle.java:178)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1668 modified TestProcessorLifecycle to en...

[apiri]

Github user apiri commented on the pull request:

https://github.com/apache/nifi/pull/324#issuecomment-211491167
  
Actually ran into this error myself on both Travis and the efforts of 
https://issues.apache.org/jira/browse/NIFI-1654 for Appveyor.  I found that 
using JUnit temp directories as per a4ef5a1badf0ae475acb1ba73867702aa5f89c80 
seemed to remedy this, but then ran into other, similar problems later on.  Not 
sure exactly what the trigger is, but it seems to be fairly consistent across 
Windows.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1668 modified TestProcessorLifecycle to en...

[olegz]

Github user olegz commented on the pull request:

https://github.com/apache/nifi/pull/324#issuecomment-211487325
  
@pvillard31 any idea why? I mean 'target' directory should be deletable. 
Just wonder if you have any idea/pointers?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request: NIFI-1668 modified TestProcessorLifecycle to en...

[pvillard31]

Github user pvillard31 commented on the pull request:

https://github.com/apache/nifi/pull/324#issuecomment-208369542
  
@olegz Tests on master are OK but not when testing this PR.
Windows 10 / Maven 3.3.9 : Java 1.0.8_74

```
---
 T E S T S
---
Running TestSuite
Tests run: 182, Failures: 29, Errors: 0, Skipped: 1, Time elapsed: 135.132 
sec <<< FAILURE! - in TestSuite
validateSuccessfullAndOrderlyShutdown on 
validateSuccessfullAndOrderlyShutdown(org.apache.nifi.controller.scheduling.TestProcessorLifecycle)(org.apache.nifi.controller.scheduling.T
estProcessorLifecycle)  Time elapsed: 5.267 sec  <<< FAILURE!
java.io.IOException: Unable to delete file: .\target\test-repo\wali.lock
at org.apache.commons.io.FileUtils.forceDelete(FileUtils.java:2279)
at org.apache.commons.io.FileUtils.cleanDirectory(FileUtils.java:1653)
at org.apache.commons.io.FileUtils.deleteDirectory(FileUtils.java:1535)
at 
org.apache.nifi.controller.scheduling.TestProcessorLifecycle.after(TestProcessorLifecycle.java:90)

validateStopCallsAreMeaninglessIfProcessorNotStarted on 
validateStopCallsAreMeaninglessIfProcessorNotStarted(org.apache.nifi.controller.scheduling.TestProcessorLifecycle)(org.apach
e.nifi.controller.scheduling.TestProcessorLifecycle)  Time elapsed: 0.001 
sec  <<< FAILURE!
java.lang.RuntimeException: java.nio.channels.OverlappingFileLockException
at sun.nio.ch.SharedFileLockTable.checkList(FileLockTable.java:255)
at sun.nio.ch.SharedFileLockTable.add(FileLockTable.java:152)
at sun.nio.ch.FileChannelImpl.lock(FileChannelImpl.java:1063)
at java.nio.channels.FileChannel.lock(FileChannel.java:1053)
at 
org.wali.MinimalLockingWriteAheadLog.(MinimalLockingWriteAheadLog.java:179)
at 
org.wali.MinimalLockingWriteAheadLog.(MinimalLockingWriteAheadLog.java:108)
at 
org.apache.nifi.controller.repository.WriteAheadFlowFileRepository.initialize(WriteAheadFlowFileRepository.java:139)
at 
org.apache.nifi.controller.FlowController.createFlowFileRepository(FlowController.java:547)
at 
org.apache.nifi.controller.FlowController.(FlowController.java:411)
at 
org.apache.nifi.controller.FlowController.createStandaloneInstance(FlowController.java:360)
at 
org.apache.nifi.controller.scheduling.TestProcessorLifecycle.buildFlowControllerForTest(TestProcessorLifecycle.java:607)
at 
org.apache.nifi.controller.scheduling.TestProcessorLifecycle.before(TestProcessorLifecycle.java:85)

validateStopCallsAreMeaninglessIfProcessorNotStarted on 
validateStopCallsAreMeaninglessIfProcessorNotStarted(org.apache.nifi.controller.scheduling.TestProcessorLifecycle)(org.apach
e.nifi.controller.scheduling.TestProcessorLifecycle)  Time elapsed: 0.001 
sec  <<< FAILURE!
java.io.IOException: Unable to delete file: .\target\test-repo\wali.lock
at org.apache.commons.io.FileUtils.forceDelete(FileUtils.java:2279)
at org.apache.commons.io.FileUtils.cleanDirectory(FileUtils.java:1653)
at org.apache.commons.io.FileUtils.deleteDirectory(FileUtils.java:1535)
at 
org.apache.nifi.controller.scheduling.TestProcessorLifecycle.after(TestProcessorLifecycle.java:90)

validateLifecycleOperationOrderWithConcurrentCallsToStartStop on 
validateLifecycleOperationOrderWithConcurrentCallsToStartStop(org.apache.nifi.controller.scheduling.TestProcessorLi
fecycle)(org.apache.nifi.controller.scheduling.TestProcessorLifecycle)  
Time elapsed: 0.001 sec  <<< FAILURE!
java.lang.RuntimeException: java.nio.channels.OverlappingFileLockException
at sun.nio.ch.SharedFileLockTable.checkList(FileLockTable.java:255)
at sun.nio.ch.SharedFileLockTable.add(FileLockTable.java:152)
at sun.nio.ch.FileChannelImpl.lock(FileChannelImpl.java:1063)
at java.nio.channels.FileChannel.lock(FileChannel.java:1053)
at 
org.wali.MinimalLockingWriteAheadLog.(MinimalLockingWriteAheadLog.java:179)
at 
org.wali.MinimalLockingWriteAheadLog.(MinimalLockingWriteAheadLog.java:108)
at 
org.apache.nifi.controller.repository.WriteAheadFlowFileRepository.initialize(WriteAheadFlowFileRepository.java:139)
at 
org.apache.nifi.controller.FlowController.createFlowFileRepository(FlowController.java:547)
at 
org.apache.nifi.controller.FlowController.(FlowController.java:411)
at 
org.apache.nifi.controller.FlowController.createStandaloneInstance(FlowController.java:360)
at 
org.apache.nifi.controller.scheduling.TestProcessorLifecycle.buildFlowControllerForTest(TestProcessorLifecycle.java:607)
at 
org.apache.nifi.controller.scheduling.TestProcessorLifecycle.before(TestProcessorLifecycle.java:85)

validateLifecycleOperationOrderWithConcurrentCallsToStartStop on 

[GitHub] nifi pull request: NIFI-1668 modified TestProcessorLifecycle to en...

[olegz]

GitHub user olegz opened a pull request:

https://github.com/apache/nifi/pull/324

NIFI-1668 modified TestProcessorLifecycle to ensure FlowController is…

… always shut down

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/olegz/nifi NIFI-1668

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi/pull/324.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #324


commit 1f04bf52531e36f3a0cb7c02334deafbe70c6a14
Author: Oleg Zhurakousky 
Date:   2016-04-04T20:21:25Z

NIFI-1668 modified TestProcessorLifecycle to ensure FlowController is 
always shut down




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---