[GitHub] kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest Exceptions

[GitBox]

kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest 
Exceptions
URL: https://github.com/apache/fluo/issues/1000#issuecomment-394903603
 
 
   Fixed by #1035 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest Exceptions

[GitBox]

kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest 
Exceptions
URL: https://github.com/apache/fluo/issues/1000#issuecomment-392346652
 
 
   Hello guys, following back up on this I have successfully used Curator 2.12
   which has a lot of bugs fixed.
   
   I think it sounds reasonable to upgrade to this version as it was as simple
   as editing the pom. No new errors are created.
   
   However, as feared, it does not fix the issue. But it does upgrade and fix
   a lot of bugs in the dependency.
   
   On Mon, Apr 9, 2018, 2:24 PM Kenneth Mcfarland 
   wrote:
   
   > Ok I will try a clean upgrade and then apply my logic to a patch if
   > necessary. Thank you Christopher.
   >
   > On Mon, Apr 9, 2018, 2:11 PM Christopher Tubbs 
   > wrote:
   >
   >> Probably both. I think an updated curator would be reasonable, but it may
   >> not fix the problem.
   >>
   >> —
   >> You are receiving this because you authored the thread.
   >> Reply to this email directly, view it on GitHub
   >> , or 
mute
   >> the thread
   >> 

   >> .
   >>
   >
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest Exceptions

[GitBox]

kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest 
Exceptions
URL: https://github.com/apache/fluo/issues/1000#issuecomment-379898726
 
 
   Ok I will try a clean upgrade and then apply my logic to a patch if
   necessary. Thank you Christopher.
   
   On Mon, Apr 9, 2018, 2:11 PM Christopher Tubbs 
   wrote:
   
   > Probably both. I think an updated curator would be reasonable, but it may
   > not fix the problem.
   >
   > —
   > You are receiving this because you authored the thread.
   > Reply to this email directly, view it on GitHub
   > , or 
mute
   > the thread
   > 

   > .
   >
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest Exceptions

[GitBox]

kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest 
Exceptions
URL: https://github.com/apache/fluo/issues/1000#issuecomment-379498792
 
 
   Ok so its been too long since I've looked at this. I just pulled down the 
new code and ran mvn with some errors left. 
   
   I want to finish this up soon because eliminating these errors to a 
reasonable degree could help clear the waters when other errors are thrown.
   
   Should I try to upgrade curator and see how that works or keep working on 
this patch with refactoring etc or both?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest Exceptions

[GitBox]

kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest 
Exceptions
URL: https://github.com/apache/fluo/issues/1000#issuecomment-358899764
 
 
   @mikewalch @keith-turner @ctubbsii 
   
   There are a few issues I've identified that are legitimate concerns. For the 
sake of this please attention to synchronization. Learning how to do line 
inserts, not sure how. Apologies. 
   
 synchronized (this) {
   byte[] d = curatorFramework.getData().forPath(maxTsPath);
   currentTs = maxTs = LongUtil.fromByteArray(d);
 }
   
   Notice the synchronization on these objects from takeLeadership()  
**above**: non synchronized method with a synchronized block
   
 private synchronized long getTimestampsImpl(String id, int num) throws 
TException {
   
   Consider this method, it is synchronized above, 
   
   allocateTimestamp();
   
   This line is not synchronized. Follow code into this method:
   
   long newMax = Long.parseLong(new String(d)) + 1000;
   
   
   
curatorFramework.setData().withVersion(stat.getVersion()).forPath(maxTsPath,
   LongUtil.toByteArray(newMax));
   maxTs = newMax;
   
   Notice there is no synchronization on these. Object monitors and locks to do 
not inherit. This is made clear at this line earlier in the code, inside a 
synchronized method: 
   
   // do this outside of sync
   stampsHistogram.update(num);
   
   I've profiled the code over the last week, this is a race condition that is 
leading to a lot of non-deterministic build errors and failures.  isLeader is 
another volatile variable that is never locked on causing race conditions and 
exceptions.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest Exceptions

[GitBox]

kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest 
Exceptions
URL: https://github.com/apache/fluo/issues/1000#issuecomment-358899764
 
 
   @mikewalch @keith-turner @ctubbsii 
   
   There are a few issues I've identified that are legitimate concerns. For the 
sake of this please attention to synchronization.
   
 synchronized (this) {
   byte[] d = curatorFramework.getData().forPath(maxTsPath);
   currentTs = maxTs = LongUtil.fromByteArray(d);
 }
   
   Notice the synchronization on these objects from takeLeadership() : non 
synchronized method with a synchronized block
   
 private synchronized long getTimestampsImpl(String id, int num) throws 
TException {
   
   Consider this method, it is synchronized. 
   
   allocateTimestamp();
   
   This line is not synchronized. Follow code into this method:
   
   long newMax = Long.parseLong(new String(d)) + 1000;
   
   
   
curatorFramework.setData().withVersion(stat.getVersion()).forPath(maxTsPath,
   LongUtil.toByteArray(newMax));
   maxTs = newMax;
   
   Notice there is no synchronization on these. Object monitors and locks to do 
not inherit. This is made clear at this line: 
   
   // do this outside of sync
   stampsHistogram.update(num);
   
   I've profiled the code over the last week, this is a race condition that is 
leading to a lot of non-deterministic build errors and failures.  
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest Exceptions

[GitBox]

kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest 
Exceptions
URL: https://github.com/apache/fluo/issues/1000#issuecomment-357456744
 
 
   @keith-turner Recommended looking at CuratorFramework.blockUntilConnected 
which I just finished trying out and it works also. This is the change to the 
OracleServer.takeLeadership method before the call to getData that is fluently 
styled right after. 
   
   I've dropped changes to the other method that was throwing exceptions, 
OracleServer.getTimestampImpl until I understand it better. I also put this in 
a serious branch on my local repo.
   
   https://github.com/kpm1985/fluo/tree/FLUO-1000


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest Exceptions

[GitBox]

kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest 
Exceptions
URL: https://github.com/apache/fluo/issues/1000#issuecomment-357456744
 
 
   @keith-turner Recommended looking at CuratorFramework.blockUntilConnected 
which I just finished trying out and it works also. This is the change to the 
OracleServer.takeLeadership method before the call to getData that is fluently 
styled right after. 
   
   I've dropped changes to the other method that was throwing exceptions, 
OracleServer.getTimestampImpl until I understand it better.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest Exceptions

[GitBox]

kpm1985 commented on issue #1000: CuratorFramework and IntegrationTest 
Exceptions
URL: https://github.com/apache/fluo/issues/1000#issuecomment-357398391
 
 
   I've pushed and reset a few times on this branch, this has been good 
experience to expand and learn a little more about git.
   
   Anyways, with these modest changes, the only errors that are appearing 
during mvn verify are in Thrift or something I don't have forked/cloned. The 
first change sleeps on the state of the curator framework, the second sleeps on 
isLeader. Basically Fluo gets a chill pill and waits for things to finish and 
for some reason this has the effect of disappearing the error messages. 
   
   I'm doing further reading and getting to learn more about ZooKeeper and 
Curator so I can do a better job at this but this is a first analysis. 
   
   https://github.com/kpm1985/fluo/tree/CuratorErrors


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services