Hi Jun Ki. I just created https://issues.apache.org/jira/browse/S2GRAPH-68 and start to work on this. I will ask your opinion while I am working on this, so please gives any feedback and let's continue discussion.
On Fri, Apr 22, 2016 at 9:10 AM Jun Ki Kim <[email protected]> wrote: > OK, I see what you said. Sorry for my mis-understanding. > More comments makes more readability. That will be very helpful to me. > After the your job, I may also try to make another suggestion. > > Thanks for your correction! > > Best regards, > Junki Kim > 2016년 4월 21일 (목) 오후 11:58, DO YUNG YOON <[email protected]>님이 작성: > > > Apology on my bad explanation. it is always hard to explain concepts > > clearly. > > > > Point is current logic is over complicated so make others hard to > > understand. > > What I am planning is start from writing comments with explanation and > > assertion. Then ask others if it makes sense, then go for implementation. > > JunKi, How about this? > > > > What's others opinion on this? > > > > Best Regards. > > DO YUNG YOON > > > > On Wed, Apr 20, 2016 at 5:37 PM Jun Ki Kim <[email protected]> wrote: > > > > > Conflict management is the hardest logic to understand how it works. I > > > think many people may agree my opinion. > > > I don't know well about core module and don't understand well what you > > > suggested too. However I totally agree with refactoring the core > logic! I > > > very welcome to make more readable to me and more comments. > > > I will try to understand and refactor the core logic. Please give me > more > > > wisdom. ^^; > > > > > > Regards, > > > Junki Kim > > > 2016년 4월 20일 (수) 오전 9:15, DO YUNG YOON <[email protected]>님이 작성: > > > > > > > Current implementation to resolve conflicts on same snapshotEdge is > > > > overwhelmingly complicated. > > > > I am suggesting refactor this part so we can understand what is going > > on > > > > for consistency. > > > > > > > > More importantly, with positive fail probability for > > > > RPC(hbase.fail.prob=0.01), test cases actually fail. > > > > we can reproduce failure by running `sbt "project s2core" > > > > -Dconfig.file=s2rest_play/conf/test.conf test`. > > > > What we intended initially here is guarantee to resolve conflicts > even > > > some > > > > RPC into storage fail. > > > > > > > > I was fixing this issue and quickly realize current code base is way > > too > > > > complicated and not efficient. > > > > > > > > here is brief explanation( > > > > > > > > > > > > > > https://steamshon.gitbooks.io/s2graph-book/content/request_state_diagram.html > > > > ). > > > > > > > > the big picture for this process goes like below. > > > > > > > > - retry on failure > > > > -- fetch snapshotEdge as oldSN. > > > > --- merge oldSN and requesting edges and build newSN containing > > > updated > > > > state. > > > > --- lock this snapshotEdge by storing (oldSN, requesting edge as > > > > pendingEdge) using checkAndSet(statusCode 0 -> 1). > > > > --- fire RPCs for indexEdge insert + delete mutations(statusCode > 1 > > -> > > > > 2). > > > > --- fire RPCs for indexEdge degree increment > mutations(statusCode 2 > > > -> > > > > 3). > > > > --- release lock on this snapshotEdge by storing (newSN, None as > > > > pendingEdge) using CheckAndSet(statusCode 3 -> 0). > > > > > > > > I think there is no need to use CheckAndSet on release lock phase > since > > > it > > > > is guaranteed for only one thread can success to lock snapshotEdge. > > > > > > > > Also fetch snapshotEdge on ever retry produce lots of RPC on same > row. > > > > Actually, I think there is no need to fetch snapshotEdge over and > over > > on > > > > every retry, once this snapshotEdge is locked. > > > > > > > > Of course threads that failed to acquire lock, still need to fetch > > > > snapshotEdge on retry, but thread who own lock on this snapshotEdge > do > > > not > > > > need to fetch it again. it is guaranteed that no other thread changed > > > > snapshotEdge while thread who own lock is retrying on partial > failure. > > > > > > > > Also current implementation use Thread.sleep on Future and throw > > > exception > > > > inside Future(not return Future.failed(ex)) which does not seems > normal > > > in > > > > asynchronous manner. > > > > > > > > Finally there is no comments and it is very hard to follow control > flow > > > for > > > > this resolving logic. > > > > > > > > I think it is worthwhile to refactor conflict resolving logic. > > > > > > > > What do you guys think? > > > > > > > > Best Regards. > > > > DOYUNG YOON > > > > > > > > > >
