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 > > >
