Conversation
Commands can be sent with an optional map of request options:
ered:command(Pid, Cmd, #{buffer_time => 10, timeout => 5000}).
When buffer_time is non-zero, the command is buffered and a timer is
started. Buffered commands are flushed when the timer fires. If an
unbuffered command (buffer_time => 0, the default) arrives while there
are buffered commands, everything is flushed immediately. This allows
coalescing multiple commands into fewer TCP packets and TLS records.
The request options map is supported in ered:command/3,
ered:command_async/4, ered_cluster:command/4 and
ered_cluster:command_async/5. Passing a plain timeout as before
still works for backward compatibility.
bjosv
left a comment
bjosv
left a comment
There was a problem hiding this comment.
Some nits/questions but LGTM
| -type command() :: ered_command:command(). | ||
| -type reply() :: ered_client:reply() | {error, unmapped_slot | client_down}. | ||
| -type reply_fun() :: ered_client:reply_fun(). | ||
| -type req_opts() :: #{timeout => timeout(), buffer_time => non_neg_integer()}. |
There was a problem hiding this comment.
For opt() we define that in ered_client:opt(), maybe we should define req_opts() in ered_client:req_opts() as well
There was a problem hiding this comment.
Yeah... I don't really know what's the best style to use.
If we define types as alias of types in other modules that are not part of the public API, then some tools that generate documentation (exdoc, edoc, etc.) don't do this in a nice way. They will display it as an alias of ered_client:req_opts() without saying what that is, since that module is not included in the docs. In docs, I prefer to see the explicit definition rather than alias. We don't use such tools now but we might in the future.
Also when reading the code, I don't like the indirection, but it's good to avoid duplicating the types. Sometimes I think it'd be better move all types to the ered module and let the other modules use that. WDYT?
| %% - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - | ||
| -spec command(cluster_ref(), command(), key()) -> reply(). | ||
| -spec command(cluster_ref(), command(), key(), timeout()) -> reply(). | ||
| -spec command(cluster_ref(), command(), key(), timeout() | ered:req_opts()) -> reply(). |
There was a problem hiding this comment.
Maybe we should use a local type in case we want to add other req_opts in the cluster API vs single API, i.e add a -type req_opts() :: ered:req_opts(). in ered_cluster.erl and use it here.
We already have a
-type addr() :: ered:addr().
2 participants
Commands can be sent with an optional map of request options:
When buffer_time is non-zero, the command is buffered and a timer is started. Buffered commands are flushed when the timer fires. If an unbuffered command (buffer_time => 0, the default) arrives while there are buffered commands, everything is flushed immediately. This allows coalescing multiple commands into fewer TCP packets and TLS records.
The request options map is supported in ered:command/3, ered:command_async/4, ered_cluster:command/4 and ered_cluster:command_async/5. Passing a plain timeout as before still works for backward compatibility.