Cross-platform support #119

AshfordN · 2020-02-23T23:42:49Z

No description provided.

freeekanayaka · 2020-02-24T11:49:22Z

I assume this is still a work in progress?

AshfordN · 2020-02-25T00:32:06Z

yeah, I'm actually stuck at trying to get this linked with libuv

paulstuart · 2020-02-25T02:27:39Z

@AshfordN The "My" prefix for disambiguation might be better as "Raft". Just a thought.

Would also like to get some darwin love in on these changes but I guess that can happen with a followup after this lands.

codecov-io · 2020-02-29T15:34:27Z

Codecov Report

Merging #119 into master will decrease coverage by 0.02%.
The diff coverage is 76.57%.

@@            Coverage Diff             @@
##           master     #119      +/-   ##
==========================================
- Coverage    86.1%   86.08%   -0.03%     
==========================================
  Files          47       47              
  Lines        7436     6892     -544     
  Branches     1166     1142      -24     
==========================================
- Hits         6403     5933     -470     
+ Misses       1028      959      -69     
+ Partials        5        0       -5

Impacted Files	Coverage Δ
src/uv_os.c	`94.59% <ø> (-1.8%)`	⬇️
src/byte.c	`98.54% <ø> (-0.04%)`	⬇️
src/recv.c	`81.39% <100%> (+0.34%)`	⬆️
src/uv_send.c	`92.66% <100%> (-0.17%)`	⬇️
src/replication.c	`79.73% <100%> (+1.71%)`	⬆️
src/heap.c	`100% <100%> (ø)`	⬆️
src/recv_append_entries.c	`86% <100%> (-1.04%)`	⬇️
src/uv_finalize.c	`67.14% <50%> (-14.2%)`	⬇️
src/raft.c	`83.07% <50%> (+1.91%)`	⬆️
src/uv_truncate.c	`75.9% <57.14%> (+1.99%)`	⬆️
... and 51 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b1cd439...eff2176. Read the comment docs.

freeekanayaka · 2020-03-26T11:45:09Z

What's the status of this branch? Is it ready for review and possibly landing?

freeekanayaka · 2020-03-30T09:53:30Z

What's the status of this branch? Is it ready for review and possibly landing?

@AshfordN ? ^^^

AshfordN · 2020-03-30T12:58:23Z

Hey,
I apologise for my inactivity on this branch but I've been a bit busy lately. At the moment, I can't seem to get this linked with libuv despite my best efforts. Although I must admit that I'm actually cross-compiling with MinGW on Linux; which might not be the ideal environment for the ground work that I'm doing.
Aside from that, there were some low-level memory handling here that could not be ported 1-to-1 (see this article).
I'd appreciate it if someone could reviewed what I have so far and make the necessary recommendations.

freeekanayaka · 2020-03-30T13:24:54Z

Hey,
I apologise for my inactivity on this branch but I've been a bit busy lately.

No worries!

Aside from that, there were some low-level memory handling here that could not be ported 1-to-1 (see this article).

The article itself has this comment at the bottom:

The Universal C Runtime that is provided with Visual Studio via the Windows 10 SDK does not include support for aligned_alloc. Instead, _aligned_malloc and _aligned_free must be used instead.

So can't use use _aligned_malloc as documented here?

I'd appreciate it if someone could reviewed what I have so far and make the necessary recommendations.

Not sure I'll be able to provide much feedback, since I'm not familiar with Windows. From my point of view, as long as the code works on Windows (something which I trust you to test) and does not break Linux (something which will be tested automatically by Travis), I've no objection merging your work.

AshfordN · 2020-03-30T17:43:28Z

So can't you use _aligned_malloc as documented here?

It's not as straight forward. I made a short comment in my version of heap.c about this issue. Currently all memory is freed with a call to free(). However, memory allocated with _aligned_malloc() needs to be freed with _aligned_free(), so there needs to be extra bookkeeping in order to keep things safe, and I'm not quite sure how to attach that extra bookkeeping at such a low level.
I left it half-done for now since I didn't want to make major changes to the code structure.
I'll attempt to bring this to review stage sometime this week.

freeekanayaka · 2020-03-30T19:30:40Z

So can't you use _aligned_malloc as documented here?

It's not as straight forward. I made a short comment in my version of heap.c about this issue. Currently all memory is freed with a call to free(). However, memory allocated with _aligned_malloc() needs to be freed with _aligned_free(), so there needs to be extra bookkeeping in order to keep things safe, and I'm not quite sure how to attach that extra bookkeeping at such a low level.
I left it half-done for now since I didn't want to make major changes to the code structure.
I'll attempt to bring this to review stage sometime this week.

Ah I see. There are very few cases were aligned_malloc is needed (basically just for direct I/O), it should not be too bad to special-case them and use a dedicated free().

Kerollmops · 2020-04-18T17:19:03Z

src/uv_os.c

+#ifdef _WIN32
+int UvOsFallocate(uv_file fd, off_t offset, off_t len)
+{
+    //TODO: implement functionality
+    return 0;
+}
+#else


It seems like posix_fallocate is also missing on OSX, I found out that mozilla is faking it by using ftruncate under the hood.

https://stackoverflow.com/a/11497568/1941280

EDIT: I found a better implementation, even if I can't find why I can't bootstrap the raft on OSX, probably due to this function.

EDIT: It seems to work on OSX now, thanks to your PR, thank you!

There's now a:

static int uvOsFallocateEmulation(int fd, off_t offset, off_t len)

in uv_os.c that can be used to emulate posix_fallocate.

unfortunately, statfs and pwrite used by uvOsFallocateEmulation are not available in mingw.

freeekanayaka · 2020-05-29T14:46:21Z

Hey @AshfordN / @Kerollmops , what's the status of this PR?

As far as I can tell, with the introduction of raft_heap->aligned_free() and the internal posix_fallocate emulation code there road should now be paved for completing this work.

AshfordN · 2020-06-04T02:15:52Z

Hey,
My main challenge with this is actually completing the build process. For some reason, MinGW isn't linking libuv and I'm not sure why. Nevertheless, I'm still able to complete the build process when targeting Linux, so I don't think I broke the build script. Work has been preventing me from pinning down the issue so I'd appreciate it if someone more familiar with windows could point out any shortcomings.
As you said, everything seem to be in place to complete the work on this PR, but since I'm unable to test it, I can't say for sure.

@Kerollmops am I to understand that your branch is working on OSX?

AshfordN · 2020-06-04T02:27:42Z

@freeekanayaka also, does my fallback code here look sensible?

freeekanayaka · 2020-06-04T07:13:40Z

@freeekanayaka also, does my fallback code here look sensible?

That might be enough in practice, although it does not completely match the current semantics: for example UvWriterClose currently waits for in-flight write requests to complete, before firing the given UvWriterCloseCb. Also, you'll need to put in place some more code to fire the UvWriterReqCb and UvWriterCloseCb.

Considering that the current implementation already has fallback support for performing the writes using libuv's threadpool (which is exactly how the uv_fs_write API you are using is implemented internally in libuv), then I'd suggest to disable the non-portable code (eventfd/io_submit) using #if/#else. You'll need to put in place more #if/#else dance, but you'll be able to leverage the existing fallback implementation without essentially re-inventing it.

Kerollmops · 2020-06-04T08:50:44Z

As far as I can tell, with the introduction of raft_heap->aligned_free() and the internal posix_fallocate emulation code there road should now be paved for completing this work.

It seems to do the work but I didn't tried it, I didn't tried to create a wrapper of this API around the Rust Allocator API, sorry. I will neither be able to test that in a near futur, but in my understading this aligned_free function was the only missing one.

@Kerollmops am I to understand that your branch is working on OSX?

I wouldn't say that it works but at least it compiles and kind of run until it locks or something. No more messages are passed between peer. It is really hard to debug this kind of stuff and don't have much time to allocate to it :/

AshfordN · 2020-06-05T15:54:32Z

I wouldn't say that it works but at least it compiles and kind of run until it locks or something. No more messages are passed between peer.

This might be due to the questionable implementation of the fallback code discussed above. The fix might not be too difficult based on what @freeekanayaka said, but I'd have to figure out the build process so that I can at least test the code. Unfortunately I can't give a definite timeline for this PR, but I will try to complete the work.

jonahharris · 2020-08-01T02:03:08Z

Has there been any non-public work on this lately? While I don't have a lot of time, it would be cool to get this running on OSX. I have it compiled and kind-of working... but I definitely don't trust it.

freeekanayaka · 2020-08-01T06:58:44Z

Has there been any non-public work on this lately?

Not that I'm aware of.

gabriel-samfira

I managed to cross compile this branch, with a few changes using mingw. It seems to work. The changes I made can be viewed here:

https://github.com/gabriel-samfira/raft/tree/hack

This branch rebases your changes and adds a few changes that were needed to get dqlite to build against it. The dqlite branch (if you're curious), is here:

https://github.com/gabriel-samfira/dqlite/pull/new/hack-it-to-bits

I have to mention that my C skills are extremely lacking. I essentially learned enough C to hack this with an axe in an attempt to get it to build with mingw. The goal was not to make a merge-able PR. So as you go through my branch(es), prepare to cringe...a lot.

gabriel-samfira · 2020-11-19T21:23:29Z

configure.ac

 # The libuv raft_io implementation is built by default if libuv is found, unless
 # explicitely disabled.
 AC_ARG_ENABLE(uv, AS_HELP_STRING([--disable-uv], [do not build the libuv-based raft_io implementation]))
 AS_IF([test "x$enable_uv" != "xno"],
-      [PKG_CHECK_MODULES(UV, [libuv >= 1.8.0], [have_uv=yes], [have_uv=no])],
+      [


This change is not needed. The reason you've had a hard time linking raft to libuv is because there is a bug in the pkg-config wrapper for cross-building. There is a discussion about it here:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=930492

It should be fixed by this:

https://salsa.debian.org/mingw-w64-team/mingw-w64/commit/17f8832bff1d2e6eadfb878f7d5afb056d200364

but that change does not seem to be available in Focal.

In any case, you can get around it by simply doing:

export PKG_CONFIG_LIBDIR=/usr/x86_64-w64-mingw32/lib/

gabriel-samfira · 2020-11-19T21:24:26Z

configure.ac

@@ -101,6 +123,7 @@ CC_CHECK_CFLAGS_APPEND([-Wendif-labels])
 CC_CHECK_CFLAGS_APPEND([-Wdate-time])
 CC_CHECK_CFLAGS_APPEND([-Wnested-externs])
 CC_CHECK_CFLAGS_APPEND([-Wconversion])
+AS_CASE([$host_os],[mingw*],[CC_CHECK_CFLAGS_APPEND([-D__USE_MINGW_ANSI_STDIO])])


Aparently, using -D__USE_MINGW_ANSI_STDIO is deprecated. I added:

case $host in *mingw*) # -D__USE_MINGW_ANSI_STDIO is deprecated it seems. CC_CHECK_FLAGS_APPEND([AM_CFLAGS],[CFLAGS],[-D_POSIX]) ;; esac

to get rid of the formatting warnings.

gabriel-samfira · 2020-11-19T21:28:17Z

src/uv_os.c

+#ifdef _WIN32
+int UvOsFallocate(uv_file fd, off_t offset, off_t len)
+{
+    //TODO: implement functionality
+    return 0;
+}
+#else


unfortunately, statfs and pwrite used by uvOsFallocateEmulation are not available in mingw.

ericcurtin · 2020-11-24T16:27:17Z

Interesting PR, tempting to crack open my Windows development environment which has been lying dormant for over a year now. clang is also worth a consideration for windows, it compiles native Windows binaries now (though I think you still need to use a different linker, unless that's changed since I last looked at it). Although mingw is a nice alternative also, clang is more native. If you compile directly on Windows also, it might make it easier to run the test suite on Windows.

AshfordN added 5 commits February 23, 2020 19:36

defines endianness for x86_64 systems

3a318cb

Support for windows

927cfcb

minor changes to the windows support (incomplete)

ce9fbc9

added TODO tag for future reference

a713471

minor change

0aecd67

Improved build process for windows

c911dce

minor changes

3be9bac

AshfordN added 2 commits February 24, 2020 23:10

minor refactoring

083477b

minor fixes

eff2176

minor fix

c623459

AshfordN changed the title ~~defines endianness for x86_64 systems~~ Cross-platform support Mar 20, 2020

freeekanayaka mentioned this pull request Apr 17, 2020

It seems that syscall is only required in uv_*.c #125

Closed

Kerollmops mentioned this pull request Apr 18, 2020

Progress of a Rust wrapper #130

Closed

Kerollmops reviewed Apr 18, 2020

View reviewed changes

freeekanayaka mentioned this pull request Apr 20, 2020

Add raft_aligned_free() API for releasing aligned memory #132

Merged

freeekanayaka mentioned this pull request Jun 27, 2020

Would dqlite support non-Linux platform? canonical/dqlite#235

Closed

gabriel-samfira reviewed Nov 19, 2020

View reviewed changes

AshfordN marked this pull request as draft February 8, 2021 16:20

rabits mentioned this pull request Feb 20, 2021

Build on MacOS #173

Closed

ralight mentioned this pull request Jan 28, 2022

Rename Heap*() to RaftHeap*() for Windows compatibility. #260

Merged

cole-miller closed this Nov 28, 2023

cole-miller mentioned this pull request Oct 22, 2024

build_mac & build_windows canonical/dqlite#705

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cross-platform support #119

Cross-platform support #119

AshfordN commented Feb 23, 2020

freeekanayaka commented Feb 24, 2020

AshfordN commented Feb 25, 2020

paulstuart commented Feb 25, 2020

codecov-io commented Feb 29, 2020

freeekanayaka commented Mar 26, 2020

freeekanayaka commented Mar 30, 2020

AshfordN commented Mar 30, 2020

freeekanayaka commented Mar 30, 2020

AshfordN commented Mar 30, 2020

freeekanayaka commented Mar 30, 2020

Kerollmops Apr 18, 2020 •

edited

Loading

freeekanayaka May 29, 2020

gabriel-samfira Nov 19, 2020

freeekanayaka commented May 29, 2020

AshfordN commented Jun 4, 2020

AshfordN commented Jun 4, 2020

freeekanayaka commented Jun 4, 2020

Kerollmops commented Jun 4, 2020

AshfordN commented Jun 5, 2020 •

edited

Loading

jonahharris commented Aug 1, 2020

freeekanayaka commented Aug 1, 2020

gabriel-samfira left a comment

gabriel-samfira Nov 19, 2020

gabriel-samfira Nov 19, 2020

gabriel-samfira Nov 19, 2020

ericcurtin commented Nov 24, 2020

Cross-platform support #119

Cross-platform support #119

Conversation

AshfordN commented Feb 23, 2020

freeekanayaka commented Feb 24, 2020

AshfordN commented Feb 25, 2020

paulstuart commented Feb 25, 2020

codecov-io commented Feb 29, 2020

Codecov Report

freeekanayaka commented Mar 26, 2020

freeekanayaka commented Mar 30, 2020

AshfordN commented Mar 30, 2020

freeekanayaka commented Mar 30, 2020

AshfordN commented Mar 30, 2020

freeekanayaka commented Mar 30, 2020

Kerollmops Apr 18, 2020 • edited Loading

Choose a reason for hiding this comment

freeekanayaka May 29, 2020

Choose a reason for hiding this comment

gabriel-samfira Nov 19, 2020

Choose a reason for hiding this comment

freeekanayaka commented May 29, 2020

AshfordN commented Jun 4, 2020

AshfordN commented Jun 4, 2020

freeekanayaka commented Jun 4, 2020

Kerollmops commented Jun 4, 2020

AshfordN commented Jun 5, 2020 • edited Loading

jonahharris commented Aug 1, 2020

freeekanayaka commented Aug 1, 2020

gabriel-samfira left a comment

Choose a reason for hiding this comment

gabriel-samfira Nov 19, 2020

Choose a reason for hiding this comment

gabriel-samfira Nov 19, 2020

Choose a reason for hiding this comment

gabriel-samfira Nov 19, 2020

Choose a reason for hiding this comment

ericcurtin commented Nov 24, 2020

Kerollmops Apr 18, 2020 •

edited

Loading

AshfordN commented Jun 5, 2020 •

edited

Loading