Moon, It’s there, waiting to be merged https://github.com/apache/incubator-zeppelin/pull/40
On 16 Apr 2015, at 03:50, moon soo Lee <[email protected]> wrote: > Hi, > > Thanks for investigating test failure and very nice explanation about the > problem. > I've just quickly read your proposed code and it looks really fine. > It'll be really much appreciated If you make a pullrequest :-) > > Thanks, > moon > > On Thu, Apr 16, 2015 at 5:14 AM Eugene Morozov <[email protected]> wrote: > >> Hi! >> >> Right after git clone today (my head commit is >> 1df116fe4a44fc5d3712817ea725a872d9d6402e) I’ve tried to build zeppelin, but >> zeppelin-interpreter fails. I digged into it and discovered the following. >> The proposal’s commit can be seen in my github branch: >> https://github.com/fathersson/incubator-zeppelin/commit/0e5a2f4d958fe35e88d9c2bb57f05ae019a3d5d7 >> >> Some insights of what I discovered. >> There is an RemoteInterpreterEventPoller, which is a Thread and while it’s >> running it constantly gets and release clients, so with a little of debug >> messages it looks like: >> before getClient: 0 >> after getClient: 1 >> before release: 1 >> after release: 0 >> before getClient: 0 >> after getClient: 1 >> before release: 1 >> after release: 0 >> >> But then, in the test RemoteInterpreterProcessTest.testClientFactory() >> after it creates RemoteInterpreterProcess and references it (thus starts >> that poller), it gets another client. So, after this call >> RemoteInterpreterProcess would contain either 1 or 2 clients at the same >> time. It just matter of timing if the following will pass or not: >> assertEquals(1, rip.getNumActiveClient()); >> >> Just out of the code itself, I’d propose to set new >> RemoteInterpreterEventPoller(interpreterGroup, this) from outside and then >> start it in the reference method. That helps to test it as it’s possible to >> provide any Thread (including mock(theReal.class)) that would do nothing, >> but not sure if it’s the best - don’t yet understand the code good enough. >> See the link above. >> >> But of what I see, the whole test basically checks if GenericObjectPool >> (commons-pool2:2.3) is actually working. I bet apache has pretty thorough >> testing for it. >> >> Eugene Morozov >> [email protected] Eugene Morozov [email protected]
