Raft API redesign - include/raft/uv.h (2/3)

[MathieuBordere]

The goal is to redesign the API so that it is easily extensible in the future without breaking clients and without the need to bump the major soversion. I will create an issue for every single of the 3 exposed header files, this is the issue for

https://github.com/canonical/raft/blob/master/include/raft/uv.h

I have taken over the header file below, removed some comments for brevity and added the changes I propose, that are the subject of this discussion, in between the code blocks.

#include <uv.h>

#include "../raft.h"

struct raft_uv_transport;

/**
 * REMOVED
 */
RAFT_API int raft_uv_init(struct raft_io *io,
                          struct uv_loop_s *loop,
                          const char *dir,
                          struct raft_uv_transport *transport);

/**
 * Release any memory allocated internally.
 */
RAFT_API void raft_uv_close(struct raft_io *io);

/**
 * Set the block size that will be used for direct I/O.
 *
 * The default is to automatically detect the appropriate block size.
 */
RAFT_API void raft_uv_set_block_size(struct raft_io *io, size_t size);

/**
 * Set the maximum initial size of newly created open segments.
 *
 * If the given size is not a multiple of the block size, the actual size will
 * be reduced to the closest multiple.
 *
 * The default is 8 megabytes.
 */
RAFT_API void raft_uv_set_segment_size(struct raft_io *io, size_t size);

/**
 * Turn snapshot compression on or off.
 * Returns non-0 on failure, this can e.g. happen when compression is requested
 * while no suitable compression library is found.
 *
 * By default snapshots are compressed if the appropriate libraries are found.
 */
RAFT_API int raft_uv_set_snapshot_compression(struct raft_io *io, bool compressed);

/**
 * Set how many milliseconds to wait between subsequent retries when
 * establishing a connection with another server. The default is 1000
 * milliseconds.
 */
RAFT_API void raft_uv_set_connect_retry_delay(struct raft_io *io, unsigned msecs);

/**
 * Emit low-level debug messages using the given tracer.
 */
RAFT_API void raft_uv_set_tracer(struct raft_io *io,
                                 struct raft_tracer *tracer);

/**
 * Enable or disable auto-recovery on startup. Default enabled.
 */
RAFT_API void raft_uv_set_auto_recovery(struct raft_io *io,
                                        bool flag);

/**
 * Callback invoked by the transport implementation when a new incoming
 * connection has been established.
 *
 * No references to @address must be kept after this function returns.
 *
 * Ownership of @stream is transferred to user code, which is responsible of
 * uv_close()'ing it and then releasing its memory.
 */
typedef void (*raft_uv_accept_cb)(struct raft_uv_transport *t,
                                  raft_id id,
                                  const char *address,
                                  struct uv_stream_s *stream);

/**
 * Callback invoked by the transport implementation after a connect request has
 * completed. If status is #0, then @stream will point to a valid handle, which
 * user code is then responsible to uv_close() and then release.
 */
struct raft_uv_connect;
typedef void (*raft_uv_connect_cb)(struct raft_uv_connect *req,
                                   struct uv_stream_s *stream,
                                   int status);

/**
 * Handle to a connect request.
 */
struct raft_uv_connect
{
    void *data;            /* User data */
    raft_uv_connect_cb cb; /* Callback */
};

/**
 * Callback invoked by the transport implementation after a close request is
 * completed.
 */
typedef void (*raft_uv_transport_close_cb)(struct raft_uv_transport *t);

/**
 * Interface to establish outgoing connections to other Raft servers and to
 * accept incoming connections from them.
 */
struct raft_uv_transport
{
    /**
     * User defined data.
     */
    void *data;

    /**
     * Implementation-defined state.
     */
    void *impl;

    /**
     * Human-readable message providing diagnostic information about the last
     * error occurred.
     */
    char errmsg[RAFT_ERRMSG_BUF_SIZE];

    /**
     * Initialize the transport with the given server's identity.
     */
    int (*init)(struct raft_uv_transport *t, raft_id id, const char *address);

    /**
     * Start listening for incoming connections.
     *
     * Once a new connection is accepted, the @cb callback passed in the
     * initializer must be invoked with the relevant details of the connecting
     * Raft server.
     */
    int (*listen)(struct raft_uv_transport *t, raft_uv_accept_cb cb);

    /**
     * Connect to the server with the given ID and address.
     *
     * The @cb callback must be invoked when the connection has been established
     * or the connection attempt has failed. The memory pointed by @req can be
     * released only after @cb has fired.
     */
    int (*connect)(struct raft_uv_transport *t,
                   struct raft_uv_connect *req,
                   raft_id id,
                   const char *address,
                   raft_uv_connect_cb cb);

    /**
     * Close the transport.
     *
     * The implementation must:
     *
     * - Stop accepting incoming connections. The @cb callback passed to @listen
     *   must not be invoked anymore.
     *
     * - Cancel all pending @connect requests.
     *
     * - Invoke the @cb callback passed to this method once it's safe to release
     *   the memory of the transport object.
     */
    void (*close)(struct raft_uv_transport *t, raft_uv_transport_close_cb cb);
};

I propose to version struct raft_uv_transport, like struct raft_fsm and struct raft_io, and add future members to the end.

No changes after this point.

/**
 * Init a transport interface that uses TCP sockets.
 */
RAFT_API int raft_uv_tcp_init(struct raft_uv_transport *t,
                              struct uv_loop_s *loop);

/**
 * Release any memory allocated internally.
 */
RAFT_API void raft_uv_tcp_close(struct raft_uv_transport *t);

/**
 * Set the IP address and port that the listening socket will bind to.
 *
 *
 * Must be called before raft_init().
 */
RAFT_API int raft_uv_tcp_set_bind_address(struct raft_uv_transport *t, const char *address);

[freeekanayaka]

This one seems easy, the proposed change sounds good to me.

[freeekanayaka]

The idea would be that the user must set struct raft_uv_transport->version before calling raft_uv_tcp_init() (or whatever other custom implementation they have), right?

Similarly, the user must set struct raft_io->version before calling raft_uv_init().

[MathieuBordere]

Closed by #303 .