ashwanthkumar / suuchi

सूचि - Toolkit to build Distributed Data Systems
https://ashwanthkumar.github.io/suuchi/
53 stars 12 forks source link

Fixing Routing bug #51

Closed ashwanthkumar closed 7 years ago

ashwanthkumar commented 7 years ago

Today we don't re-try on forward failures. Assume we've 3 nodes - A,B and C. Client has done some puts and with replication of 2, the data is distributed and replicated between all the 3 nodes. Assume now A goes down. We should still be able to serve all requests from other nodes in the cluster because of replication.

This change would make sure we re-try all the nodes until one of them succeeds else we would return FAILED_PRECONDITION response to the client.

Credits to @codingnirvana for helping me discover this bug.

ashwanthkumar commented 7 years ago

@brewkode Please check this.

codecov-io commented 7 years ago

Current coverage is 61.61% (diff: 48.00%)

Merging #51 into master will increase coverage by 1.27%

@@             master        #51   diff @@
==========================================
  Files            23         23          
  Lines           411        409     -2   
  Methods           0          0          
  Messages          0          0          
  Branches         63         68     +5   
==========================================
+ Hits            248        252     +4   
+ Misses          127        117    -10   
- Partials         36         40     +4   

Sunburst

Powered by Codecov. Last update d710d4e...6792e66

ashwanthkumar commented 7 years ago

@brewkode I've also updated the ExampleApp to make it easier to run it as a multi-process example on different ports. This enables us to test scenarios like the above which otherwise we always took for granted. Please share your thoughts on this approach. We can also update the DistributedRocksDb example also to be like this.

Also we should look at what's the even better way to write Integration Tests or some other approach to catch these pro-actively. Let me know if you have some thoughts around this too.

ashwanthkumar commented 7 years ago

@brewkode Merging this too for cutting out a release.