Learner node #210

delamonpansie · 2018-01-23T12:21:44Z

No description provided.

toolslive · 2018-01-23T12:29:09Z

pylabs/Compat.py

@@ -144,7 +144,7 @@ def copyDirTree(self,src,dst):

    root = os.environ.get('TEST_HOME')
    if root is None:
-        root = os.environ['HOME']
+        raise Exception("TEST_HOME is undefined")


impact on travis/jenkins?

I have no idea, but after cleaning my home directory several times I believe this is the right thing to do.

toolslive · 2018-01-23T12:41:13Z

src/paxos/learner.ml

@@ -0,0 +1,203 @@
+(*
+Copyright (2010-2014) INCUBAID BVBA


we should probably move to a copyright notice like the one in Alba. I think it's not acceptable to attribute copyright to someone who didn't create it.

toolslive · 2018-01-23T12:42:54Z

src/paxos/learner.ml

+  let sides =
+    if is_election constants
+    then [log_e; EStartElectionTimeout (current_n, current_i); mcast_e]
+    else [log_e; mcast_e]


factor out common [log_e; mcast_e] ?

toolslive · 2018-01-23T12:45:31Z

src/paxos/learner.ml

+      end
+    | ElectionTimeout _ | LeaseExpired _ ->
+       let log_e = ELog (fun () ->
+                       Printf.sprintf "steady_state: ignoring lease expiration because I am a learner")


sprintf not needed.

toolslive · 2018-01-23T12:47:19Z

src/paxos/learner.ml

+      Fsm.return (Learner_steady_state (future_n, current_i', None));
+    end
+  else
+    failwith "FIXME"


this should be impossible maybe replace with some kind of assert ?

domsj

I have a few minor comments below, but I would like to make a few more general comments too (and perhaps tomorrow have some discussion in the chat):

this (almost) duplicates a bunch of existing code
the existing (now duplicated) code contains some logic related to constants.is_learner which is now obsolete
should the consensus nodes actually know about the learner nodes? perhaps we could have a design where the learner node subscribes to the stream of updates instead (in which case it can even choose which node it wants to follow - e.g. it could follow the master, or some slave in the same datacenter)

domsj · 2018-01-23T15:53:29Z

src/paxos/multi_paxos_fsm.ml

@@ -741,6 +744,25 @@ let _execute_effects constants e =

 (* the entry methods *)

+let enter_learner ?(stop = ref false) constants buffers new_i =
+  let me = constants.me in
+  Logger.debug_f_ "%s: +starting FSM for forced_slave." me >>= fun () ->


s/forced_slave/learner?

domsj · 2018-01-23T16:01:32Z

src/paxos/multi_paxos_type.ml

+
+
+type learner_transitions =
+  | Learner_start_transition


this constructor isn't actually used anywhere and could be removed?

If we don't want to test it, then yes.

domsj · 2018-01-23T16:13:31Z

src/paxos/learner.ml

+  let mcast_e = EMCast fake in
+  let sides =
+    if is_election constants
+    then [log_e; EStartElectionTimeout (current_n, current_i); mcast_e]


Should a learner ever produce the EStartElectionTimeout action? If I'm not mistaken it will ignore the resulting ElectionTimeout event anyway...
(so I think the if can go away)

I'm suspicious about elections too. We've discussed with Romain this yesterday.

delamonpansie · 2018-01-24T09:37:21Z

should the consensus nodes actually know about the learner nodes? perhaps we could have a design where the learner node subscribes to the stream of updates instead (in which case it can even choose which node it wants to follow - e.g. it could follow the master, or some slave in the same datacenter)

this can be done, but it should not be named "learner"

delamonpansie · 2018-01-26T07:05:23Z

This is still a work in progress. Your comments are very much appreciated.

toolslive

(github can't show changes to slave.ml)

toolslive · 2018-01-26T09:17:11Z

src/paxos/learner.ml

+let handle_from_node ((n,i,previous) as state) (type s) (module S : Store.STORE with type t = s)
+      {store; me;on_witness} msg source =
+  Logger.ign_debug_f_ "Learner.steady_state n:%s i:%s v:%s store_i:%Li got %S from %s"
+    (Sn.string_of n) (Sn.string_of i) (match previous with  Some v -> "Some " ^ Value.value2s v | None -> "None")


there's To_string.option

toolslive · 2018-01-26T09:22:37Z

src/paxos/learner.ml

@@ -0,0 +1,129 @@
+(*
+  Copyright (C) iNuron - info@openvstorage.com
+  This file is part of Open vStorage. For license information, see <OVS_LICENSE.txt>


just point to the LICENSE file (the apache 2.0)

toolslive · 2018-01-26T09:22:53Z

OVS_LICENSE.TXT

@@ -0,0 +1,208 @@
+              Open vStorage License Agreement


no. all we add keeps being released under Apache 2.0. (see lower)

wimpers added the state_inprogress label Jan 23, 2018

toolslive reviewed Jan 23, 2018

View reviewed changes

domsj reviewed Jan 23, 2018

View reviewed changes

delamonpansie force-pushed the learner-node branch from bc174e3 to 8153645 Compare January 26, 2018 07:04

toolslive reviewed Jan 26, 2018

View reviewed changes

delamonpansie force-pushed the learner-node branch 3 times, most recently from 3550e17 to a1ff885 Compare January 26, 2018 12:26

toolslive approved these changes Jan 30, 2018

View reviewed changes

Yuriy Vostrikov added 13 commits January 30, 2018 15:58

Dynamic JOBS calculation

3cc12a6

Silence nonrelevant error

f4060fb

Pass -use-ocamlfind implicitly

176a0e7

Build arakoon.native before testing

a3daf4b

Remove unused read_only

2e602c2

Do not polute $HOME

d56ecce

Decrease op_count to sane value

fb4d177

Indent

a31ae73

Add comments

e325913

Factor out decline_client_update

82b4f1d

Simplify decline_client_update

3c041b7

Split slave_steady_state

926c8ea

Learner node

64a5690

delamonpansie force-pushed the learner-node branch from a1ff885 to 64a5690 Compare January 30, 2018 13:02

delamonpansie merged commit c05c167 into 1.9 Jan 31, 2018

delamonpansie deleted the learner-node branch January 31, 2018 16:48

wimpers removed the state_inprogress label Jan 31, 2018

toolslive mentioned this pull request Nov 7, 2018

Make learner node functionality available again #138

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Learner node #210

Learner node #210

delamonpansie commented Jan 23, 2018

toolslive Jan 23, 2018

delamonpansie Jan 24, 2018

toolslive Jan 23, 2018

delamonpansie Jan 24, 2018

toolslive Jan 23, 2018

toolslive Jan 23, 2018

toolslive Jan 23, 2018

domsj left a comment

domsj Jan 23, 2018

delamonpansie Jan 24, 2018

domsj Jan 23, 2018

delamonpansie Jan 24, 2018

domsj Jan 23, 2018

delamonpansie Jan 24, 2018

delamonpansie commented Jan 24, 2018

delamonpansie commented Jan 26, 2018

toolslive left a comment

toolslive Jan 26, 2018

toolslive Jan 26, 2018

toolslive Jan 26, 2018

Learner node #210

Learner node #210

Conversation

delamonpansie commented Jan 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domsj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

delamonpansie commented Jan 24, 2018

delamonpansie commented Jan 26, 2018

toolslive left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment