FLU1 Factorization 1/N: Introduce ranch and factor out socket handling process

shino commented

2015-11-19 08:52:35 +00:00

(Migrated from github.com)

This PR introduces Ranch as network transport abstraction and adds socket handling
processes ("protocol handler" in Ranch's terminology), as the first round of FLU1 factorization.

TODO for later effort: Remove machi_listener_sup if possible

Not directly related commits are included in this PR:

3bc5ec4 : set longer timeout for huge write test of machi_file_proxy_test
c3e961b , 1e42a7f, 77a1ffd : cosmetics and refactoring for initialization phase of test modules

This PR introduces Ranch as network transport abstraction and adds socket handling processes ("protocol handler" in Ranch's terminology), as the first round of FLU1 factorization. TODO for later effort: Remove machi_listener_sup _if possible_ Not directly related commits are included in this PR: - 3bc5ec4 : set longer timeout for huge write test of `machi_file_proxy_test` - c3e961b , 1e42a7f, 77a1ffd : cosmetics and refactoring for initialization phase of test modules

jadeallenx commented

2015-11-21 04:03:53 +00:00

(Migrated from github.com)

@shino This looks great so far! Nice work!

shino commented

2015-12-08 06:48:22 +00:00

(Migrated from github.com)

Ready for review, I hope 🔨

Ready for review, I hope :hammer:

jadeallenx commented

2015-12-08 07:11:16 +00:00

(Migrated from github.com)

Planning to look at this tomorrow.

slfritchie commented

2015-12-08 07:21:58 +00:00

(Migrated from github.com)

Preliminary +1 from me, yay.

shino commented

2015-12-08 07:29:12 +00:00

(Migrated from github.com)

Rebased on current master and force-pushed.

jadeallenx commented

2015-12-08 22:22:44 +00:00

(Migrated from github.com)

The only confirmed test failure was the known doublewrite problem.

I did see this message scroll by but I'm not sure if its important:

machi_flu1_test: flu_projection_smoke_test...
=ERROR REPORT==== 8-Dec-2015::16:18:06 ===
Ranch listener smoke_flu_listener had connection process started with machi_flu1_net_server:start_link/4 at <0.4450.0> exit with reason: badarg
[0.117 s] ok

I'm a little concerned the try/after block might be masking a test failure we should care about.

Otherwise, this is suuuuuper cool. 🍰

The only confirmed test failure was the known doublewrite problem. I **did** see this message scroll by but I'm not sure if its important: ``` shel machi_flu1_test: flu_projection_smoke_test... =ERROR REPORT==== 8-Dec-2015::16:18:06 === Ranch listener smoke_flu_listener had connection process started with machi_flu1_net_server:start_link/4 at <0.4450.0> exit with reason: badarg [0.117 s] ok ``` I'm a little concerned the try/after block might be masking a test failure we should care about. Otherwise, this is **suuuuuper** cool. :cake:

shino commented

2015-12-09 01:33:53 +00:00

(Migrated from github.com)

Rebased and force pushed again.

shino commented

2015-12-09 01:36:40 +00:00

(Migrated from github.com)

Ranch listener smoke_flu_listener had connection process started with machi_flu1_net_server:start_link/4 at <0.4450.0> exit with reason: badarg

I saw the line once in a while in machi_flu1_test... but not sure why.

> Ranch listener smoke_flu_listener had connection process started with machi_flu1_net_server:start_link/4 at <0.4450.0> exit with reason: badarg I saw the line once in a while in `machi_flu1_test`... but not sure why.

shino commented

2015-12-10 07:00:25 +00:00

(Migrated from github.com)

Ranch listener smoke_flu_listener had connection process started with
machi_flu1_net_server:start_link/4 at <0.4450.0> exit with reason: badarg

Identified. Race between machi_flu1 and machi_flu1_net_server at cleanup phase,
if machi_flu1 dies first, ets table has gone and net server process gets badarg.
I just added a comment at eef00e4.

> Ranch listener smoke_flu_listener had connection process started with > machi_flu1_net_server:start_link/4 at <0.4450.0> exit with reason: badarg Identified. Race between `machi_flu1` and `machi_flu1_net_server` at cleanup phase, if `machi_flu1` dies first, ets table has gone and net server process gets `badarg`. I just added a comment at eef00e4.

slfritchie commented

2015-12-10 07:20:31 +00:00

(Migrated from github.com)

Hmm, yes, if that table disappears because the FLU proc has exited (clean shutdown or crash), the 'badarg' is good behavior in that it causes the net server proc to crash and so is "safe" from a Machi operation perspective. That probably also closes the TCP connection without a reply, which isn't very polite, but better than sending nonsense?

jadeallenx commented

2015-12-10 22:42:05 +00:00

(Migrated from github.com)

👍

@shino great work! Thank you for digging in on that race condition.

:+1: @shino great work! Thank you for digging in on that race condition.

shino commented

2015-12-11 01:18:14 +00:00

(Migrated from github.com)

Thanks for review & valuable comments! > Scott & Mark

FLU1 Factorization 1/N: Introduce ranch and factor out socket handling process #46