mfp / oraft

Raft consensus algorithm implementation
Other
33 stars 5 forks source link

Fix compilation with newer OCaml and libraries #5

Closed vbmithr closed 6 years ago

vbmithr commented 6 years ago

I needed this for a project so I made it work on my current setup. The main problem was extprot that does not work with newer OCaml. I replaced with bin_prot from Janstreet, and also replaced deprecated Lwt_log to Logs. Also I changed the build system to dune.

mfp commented 6 years ago

On Mon, Oct 08, 2018 at 01:43:54AM -0700, Vincent Bernardoff wrote:

I needed this for a project so I made it work on my current setup. The main problem was extprot that does not work with newer OCaml. I replaced with bin_prot from Janstreet, and also replaced deprecated Lwt_log to Logs.

oraft is structured in 3 layers: the core state machine (module Oraft) independent from transport mechanism and concurrency machinery, a specialization of it for Lwt concurrency and a wire protocol with known extensibility (module Oraft_lwt), and a high-level replicated state machine (module RSM). The idea was to have 2 packages (say, oraft-core and oraft-lwt) at some point.

I am not keen on using bin_prot via deriving extensions directly on the core Oraft types, convenient as it is, because it introduces very tight coupling between the wire protocol (which doesn't have extensibility in mind as extprot protocols) and such types, and since Oraft is about distributed systems, this seems a recipe for future problems. It also pulls in, for the very core, the JST ecosystem that brings potential for issues with dependencies, etc. .

So I'm all for 3 out of 4 parts of the PR ("build: switch to dune", "Logs: switch to logs", and "Dict: switch to cmdliner"), but I'd prefer to keep Oraft "pure" and unconcerned about messaging (as for concurrency).

I believed extprot was 4.06-safe, but I was probably mistaking it for sqlexpr. I'm looking into it (should be some minor issue with safe-string), and that should hopefully make oraft usable on newer OCaml. Then we can rebase the commits I listed above. Does that sound OK?

-- Mauricio Fernández

mfp commented 6 years ago

On Mon, Oct 08, 2018 at 03:04:30PM +0200, Mauricio Fernández wrote:

On Mon, Oct 08, 2018 at 01:43:54AM -0700, Vincent Bernardoff wrote:

I believed extprot was 4.06-safe, but I was probably mistaking it for sqlexpr. I'm looking into it (should be some minor issue with safe-string), and that should hopefully make oraft usable on newer OCaml. Then we can rebase the commits I listed above. Does that sound OK?

OK, as expected it was just a matter of sorting out bytes/string uses:

https://github.com/mfp/extprot/commit/a0fb63fb02f5e4ca4b07ed16af4132de88370a60

With this (and the proper pin), Oraft should hopefully build OK with your commits (w/o the bin_prot one). RSM clients (now Oraft_rsm) should be unaffected a priori.

-- Mauricio Fernández

vbmithr commented 6 years ago

Ok, excellent thanks. I should have separated the bin_prot work from updating Lwt ppx syntax… That said it should not be too hard to get rid of bin_prot and replug extprot instead.

Best,

vbmithr commented 6 years ago

Or what about reexporting the core types in the Lwt part of the library?

mfp commented 6 years ago

On Tue, Oct 09, 2018 at 06:06:08AM -0700, Vincent Bernardoff wrote:

Or what about reexporting the core types in the Lwt part of the library?

That would indeed keep the Oraft core pure, but we'd still be bringing the issues at the next level (tight coupling and code rigidity, fragile wire protocol, heavy dependencies). I don't see any substantial gain to it to weight against those disadvantages.

I only had a cursory look at the PR the other day, but I assume bringing back the older wire protocol is just a matter of removing the deriving annotations, recovering some deleted (un)wrap functions and changing the couple lines that do the actual (de)serialization, isn't it?

-- Mauricio Fernández

vbmithr commented 6 years ago

On 10/11/18 5:59 AM, mfp wrote:

I only had a cursory look at the PR the other day, but I assume bringing back the older wire protocol is just a matter of removing the deriving annotations, recovering some deleted (un)wrap functions and changing the couple lines that do the actual (de)serialization, isn't it?

Something like this. Let me try… V

vbmithr commented 6 years ago

Reopened as #6