Hi JunKi. I had free time today, and start working on https://issues.apache.org/jira/browse/S2GRAPH-68. You can find out where this is going to on https://github.com/apache/incubator-s2graph/pull/53. Any feedback and question would be appreciated.
On Thu, Apr 28, 2016 at 12:33 AM DO YUNG YOON <[email protected]> wrote: > 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 >> > > > >> > > >> > >> >
