Ss/flu1 factorization2

shino commented

2015-12-16 09:04:05 +00:00

(Migrated from github.com)

Separating append server related code to new module.

jadeallenx commented

2015-12-16 20:09:02 +00:00

(Migrated from github.com)

I love these revisions - but I have a concern about casting the wedge state change message to another process. In the current FLU when wedge status changes, that state change is immediate and flows through any subsequent requests from clients until the chain manager comes along to rescue things.

It seems like we introduce a race condition by casting where the message to update the wedged state is sent, and in the meanwhile, future incoming requests are processed as if nothing were wrong. That seems like it might impact the safety guarantees we're trying to make.

I love these revisions - but I have a concern about casting the wedge state change message to another process. In the current FLU when wedge status changes, that state change is immediate and flows through any subsequent requests from clients until the chain manager comes along to rescue things. It seems like we introduce a race condition by casting where the message to update the wedged state is sent, and in the meanwhile, future incoming requests are processed as if nothing were wrong. That seems like it might impact the safety guarantees we're trying to make.

shino commented

2015-12-17 02:03:27 +00:00

(Migrated from github.com)

Thank you for looking closely @mrallen1

I'm sorry that I don't understand the comment correctly 😅 , gen_server:cast() are used to replace erlang:send() (or !) and the caller does not wait for ack/reply for it.
So, I'm wondering whether this PR changes message pattern and introduces some race / consistency breaking or there has been possibility to suffer consistency around wedge state management. Apparently, either is important 🍖 😄

Thank you for looking closely @mrallen1 I'm sorry that I don't understand the comment correctly :sweat_smile: , `gen_server:cast()` are used to replace `erlang:send()` (or `!`) and the caller does not wait for ack/reply for it. So, I'm wondering whether this PR changes message pattern and introduces some race / consistency breaking or there has been possibility to suffer consistency around wedge state management. Apparently, either is important :meat_on_bone: :smile:

jadeallenx commented

2015-12-17 21:32:39 +00:00

(Migrated from github.com)

I'm not sure I can easily explain my concerns in a comment, so maybe we could have a chat tonight on Mumble or Screenhero and I can step through my concerns - it may well be that I am misunderstanding how the original code worked.

shino commented

2015-12-18 01:05:32 +00:00

(Migrated from github.com)

maybe we could have a chat tonight on Mumble or Screenhero and I can step through my concerns

It will be very helpful, thanks!

~~As for screenhero, I tried a few weeks ago but couldn't download it~~

> maybe we could have a chat tonight on Mumble or Screenhero and I can step through my concerns It will be very helpful, thanks! <del>As for screenhero, I tried a few weeks ago but couldn't download it </del>

jadeallenx commented

2015-12-18 04:10:47 +00:00

(Migrated from github.com)

After our discussion, I think these changes are equivalent in behavior to the original implementation. My mistake in misremembering how they were structured.

👍

After our discussion, I think these changes are equivalent in behavior to the original implementation. My mistake in misremembering how they were structured. :+1:

slfritchie commented

2015-12-18 08:19:14 +00:00

(Migrated from github.com)

+1, many thanks!

Ss/flu1 factorization2 #55