diff --git a/.clang-format.example b/.clang-format.example new file mode 100644 index 00000000..fa3391b2 --- /dev/null +++ b/.clang-format.example @@ -0,0 +1,45 @@ +# This may be useful with https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting +# but some of clang's reformattings may conflict with notes/c-style-guide.txt, which takes precedence. +# +# This is deliberately named .clang-format.example instead of .clang-format to avoid editors +# unexpectedly using this to reformatting entire files because of these shortcomings. +# It can be copied into .clang-format to use this for local development selectively. +# +# Note: this clang format file has many shortcomings. +# Attempting to apply this automatically to everything may make code less readable. +# However, this may be useful for spot checking new code, +# or if you're not certain of indentation style or general spacing/wrapping rules +# +# Known shortcomings: +# - Some places exceed the 80 column limit deliberately for readability, e.g. help strings or error messages or function prototypes. (BreakStringLiterals helps preserve some of those) +# - Some places deliberately put blocks on a single line when there are a lot of similar blocks. +# AllowShortBlocksOnASingleLine is not useful. +# - Some places deliberately put blocks on a single line when there are a lot of similar blocks. +# - No good way to eliminate space before and after PRIu64 and other macros for adjacent string literal concatenation +# - clang-format is not aware of macros, some of which have different styles from functions. +# - Function declarations are not typically aligned +# - Some variable declarations are aligned and others aren't on a case by case basis +# - The choice of function argument grouping should depends on which function arguments are semantically related, +# not just on fitting within 80 columns. + +AllowShortBlocksOnASingleLine: true +BasedOnStyle: LLVM +AlwaysBreakAfterDefinitionReturnType: All +UseTab: Never +IndentWidth: 4 +TabWidth: 4 +ColumnLimit: 80 +BreakBeforeBraces: Linux +SortIncludes: false + +BreakStringLiterals: false +# BitFieldColonSpacing is too new to work in clang-format 11 +# https://releases.llvm.org/11.0.0/tools/clang/docs/ClangFormatStyleOptions.html +# Latest: https://clang.llvm.org/docs/ClangFormatStyleOptions.html +# +# BitFieldColonSpacing: None +# +# XXX no way to treat the `*` indicating a value is a pointer as part of the aligned name for the declaration +# XXX function declarations are not typically aligned +AlignConsecutiveDeclarations: true +AlignConsecutiveMacros: true diff --git a/.dockerignore b/.dockerignore index 1e41ae2b..6d7c05e4 100644 --- a/.dockerignore +++ b/.dockerignore @@ -51,7 +51,7 @@ core* nutcracker # extracted yaml -!/contrib/yaml-0.1.4.tar.gz +!/contrib/yaml-0.2.5.tar.gz # Autotools .deps diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 00000000..73461bc7 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,19 @@ +# https://editorconfig.org/ + +root = true + +[*] +trim_trailing_whitespace = true +insert_final_newline = true +end_of_line = lf +charset = utf-8 + +# See twemproxy/notes/c-styleguide.txt +[*.c,*.h] +tab_width = 4 +indent_size = 4 +indent_style = space +# indent_brace_style depends on function vs conditional +max_line_length = 80 +spaces_around_brackets = none +spaces_around_operators = true diff --git a/.gitignore b/.gitignore index 2559c013..c4002436 100644 --- a/.gitignore +++ b/.gitignore @@ -41,7 +41,7 @@ core* nutcracker # extracted yaml -!/contrib/yaml-0.1.4.tar.gz +!/contrib/yaml-0.2.5.tar.gz # Autotools .deps @@ -71,3 +71,11 @@ nutcracker Makefile Makefile.in + +# The .clang-format.example file may be copied here for use with -style=file +.clang-format + +test_all +*.trs +tags + diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 37b30783..00000000 --- a/.travis.yml +++ /dev/null @@ -1,13 +0,0 @@ -services: - - docker - -# We test the latest patch commits of multiple redis versions because sentinel responses are different, -# and to verify that unit tests of redis pass in all of those versions. -env: - - REDIS_VER=3.0.7 - - REDIS_VER=3.2.12 - - REDIS_VER=4.0.10 - -script: ./travis.sh $REDIS_VER - -sudo: required diff --git a/ChangeLog b/ChangeLog index 3ce41b1d..6c12f240 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,7 +9,6 @@ Fix memory corruption seen when all hosts are ejected from a pool with auto_eject_hosts: true (tyson) Update tests to work with python 3 (tyson) - Ensure file permissions aren't chosen based on uninitialized memory. (tyson) Add a heartbeat patch to detect dead servers (charsyam) Add improvements to the heartbeat patch (tyson) Decrease the verbosity of logging clients establishing/closing connections to @@ -32,15 +31,41 @@ server when reconnecting to a server. Reduce concurrent reconnection attempts. (tyson) 2016-04-27 Misha Nasledov - * twemproxy: version 0.5.1 release + * twemproxy: version 0.5.1 release (ifwe fork on github) updated to latest master from twitter/twemproxy updated to latest version of andyqzb's sentinel patch 2015-07-14 Misha Nasledov - * twemproxy: version 0.5.0 release + * twemproxy: version 0.5.0 release (ifwe fork on github) redis-sentinel support (andyqzb) -2015-22-06 Manju Rajashekhar + 2021-??-?? Tyson Andre + * twemproxy: version 0.5.0 release (dev) + Fix parsing of redis error response for error type with no space, + add tests (tyson, tom dalton) + Update integration tests, add C unit test suite for 'make check' (tyson) + Increase the maximum host length+port+identifier to 273 + in ketama_update (李广博) + Always initialize file permissions field when listening on a unix domain + socket (tyson) + Use number of servers instead of number of points on the continuum when + sharding requests to backend services to improve sharding performance + and fix potential invalid memory access when all hosts were ejected + from a pool. (tyson) + Don't fragment memcache/redis get commands when they only have a single + key (improves performance and error handling of single key case) (tyson) + Don't let requests hang when there is a dns error when processing a + fragmented request (e.g. multiget with multiple keys) (tyson) + Allow extra parameters for redis spop (charsyam) + Update documentation and README (various) + Fix memory leak bug for redis mset (deep011) + Support arbitrarily deep nested redis multi-bulk + responses (nested arrays) (qingping209, tyson) + Upgrade from libyaml 0.1.4 to 0.2.5 (tyson) + Fix compiler warnings about wrong conversion specifiers in format + strings for logging (tyson) + + 2015-22-06 Manju Rajashekhar * twemproxy: version 0.4.1 release backend server hostnames are resolved lazily redis_auth is only valid for a redis pool diff --git a/NOTICE b/NOTICE index efe6905d..257983f4 100644 --- a/NOTICE +++ b/NOTICE @@ -101,7 +101,7 @@ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -The source also includes libyaml (yaml-0.1.4) in contrib/ directory +The source also includes libyaml (yaml-0.2.5) in contrib/ directory Copyright (c) 2006 Kirill Simonov diff --git a/README.md b/README.md index 6c288796..999ed5aa 100644 --- a/README.md +++ b/README.md @@ -83,23 +83,23 @@ Twemproxy can be configured through a YAML file specified by the -c or --conf-fi + **listen**: The listening address and port (name:port or ip:port) or an absolute path to sock file (e.g. /var/run/nutcracker.sock) for this server pool. + **client_connections**: The maximum number of connections allowed from redis clients. Unlimited by default, though OS-imposed limitations will still apply. + **hash**: The name of the hash function. Possible values are: - + one_at_a_time - + md5 - + crc16 - + crc32 (crc32 implementation compatible with [libmemcached](http://libmemcached.org/)) - + crc32a (correct crc32 implementation as per the spec) - + fnv1_64 - + fnv1a_64 - + fnv1_32 - + fnv1a_32 - + hsieh - + murmur - + jenkins + + one_at_a_time + + md5 + + crc16 + + crc32 (crc32 implementation compatible with [libmemcached](http://libmemcached.org/)) + + crc32a (correct crc32 implementation as per the spec) + + fnv1_64 + + fnv1a_64 + + fnv1_32 + + fnv1a_32 + + hsieh + + murmur + + jenkins + **hash_tag**: A two character string that specifies the part of the key used for hashing. Eg "{}" or "$$". [Hash tag](notes/recommendation.md#hash-tags) enable mapping different keys to the same server as long as the part of the key within the tag is the same. + **distribution**: The key distribution mode. Possible values are: - + ketama (default, recommended. An implementation of https://en.wikipedia.org/wiki/Consistent_hashing) - + modula (use hash modulo number of servers to choose the backend) - + random (choose a random backend) + + ketama (default, recommended. An implementation of https://en.wikipedia.org/wiki/Consistent_hashing) + + modula (use hash modulo number of servers to choose the backend) + + random (choose a random backend) + **timeout**: The timeout value in msec that we wait for to establish a connection to the server or receive a response from a server. By default, we wait indefinitely. + **backlog**: The TCP backlog argument. Defaults to 512. + **preconnect**: A boolean value that controls if twemproxy should preconnect to all the servers in this pool on process start. Defaults to false. @@ -201,13 +201,13 @@ For example, the configuration file in [conf/nutcracker.yml](conf/nutcracker.yml - 127.0.0.1:26380:1 - 127.0.0.1:26381:1 -Finally, to make writing a syntactically correct configuration file easier, twemproxy provides a command-line argument -t or --test-conf that can be used to test the YAML configuration file for any syntax error. +Finally, to make writing a syntactically correct configuration file easier, twemproxy provides a command-line argument `-t` or `--test-conf` that can be used to test the YAML configuration file for any syntax error. ## Observability Observability in twemproxy is through logs and stats. -Twemproxy exposes stats at the granularity of server pool and servers per pool through the stats monitoring port. The stats are essentially JSON formatted key-value pairs, with the keys corresponding to counter names. By default stats are exposed on port 22222 and aggregated every 30 seconds. Both these values can be configured on program start using the -c or --conf-file and -i or --stats-interval command-line arguments respectively. You can print the description of all stats exported by using the -D or --describe-stats command-line argument. +Twemproxy exposes stats at the granularity of server pool and servers per pool through the stats monitoring port. The stats are essentially JSON formatted key-value pairs, with the keys corresponding to counter names. By default stats are exposed on port 22222 and aggregated every 30 seconds. Both these values can be configured on program start using the `-c` or `--conf-file` and `-i` or `--stats-interval` command-line arguments respectively. You can print the description of all stats exported by using the `-D` or `--describe-stats` command-line argument. $ nutcracker --describe-stats @@ -233,13 +233,13 @@ Twemproxy exposes stats at the granularity of server pool and servers per pool t out_queue "# requests in outgoing queue" out_queue_bytes "current request bytes in outgoing queue" -Logging in twemproxy is only available when twemproxy is built with logging enabled. By default logs are written to stderr. Twemproxy can also be configured to write logs to a specific file through the -o or --output command-line argument. On a running twemproxy, we can turn log levels up and down by sending it SIGTTIN and SIGTTOU signals respectively and reopen log files by sending it SIGHUP signal. +Logging in twemproxy is only available when twemproxy is built with logging enabled. By default logs are written to stderr. Twemproxy can also be configured to write logs to a specific file through the `-o` or `--output` command-line argument. On a running twemproxy, we can turn log levels up and down by sending it SIGTTIN and SIGTTOU signals respectively and reopen log files by sending it SIGHUP signal. ## Pipelining Twemproxy enables proxying multiple client connections onto one or few server connections. This architectural setup makes it ideal for pipelining requests and responses and hence saving on the round trip time. -For example, if twemproxy is proxying three client connections onto a single server and we get requests - 'get key\r\n', 'set key 0 0 3\r\nval\r\n' and 'delete key\r\n' on these three connections respectively, twemproxy would try to batch these requests and send them as a single message onto the server connection as 'get key\r\nset key 0 0 3\r\nval\r\ndelete key\r\n'. +For example, if twemproxy is proxying three client connections onto a single server and we get requests - `get key\r\n`, `set key 0 0 3\r\nval\r\n` and `delete key\r\n` on these three connections respectively, twemproxy would try to batch these requests and send them as a single message onto the server connection as `get key\r\nset key 0 0 3\r\nval\r\ndelete key\r\n`. Pipelining is the reason why twemproxy ends up doing better in terms of throughput even though it introduces an extra hop between the client and server. diff --git a/ci/Dockerfile b/ci/Dockerfile index 05b3f1d4..86e1b5dc 100644 --- a/ci/Dockerfile +++ b/ci/Dockerfile @@ -30,7 +30,7 @@ RUN pip3.6 install nose && \ # RUN yum install -y redis redis-sentinel ARG REDIS_VER=3.2.11 -RUN wget https://github.com/antirez/redis/archive/$REDIS_VER.tar.gz && \ +RUN wget https://github.com/redis/redis/archive/$REDIS_VER.tar.gz && \ tar zxvf $REDIS_VER.tar.gz && \ pushd redis-$REDIS_VER && \ make install && \ diff --git a/ci/build-nutcracker.sh b/ci/build-nutcracker.sh index 6d45e431..d54b9582 100755 --- a/ci/build-nutcracker.sh +++ b/ci/build-nutcracker.sh @@ -9,9 +9,12 @@ trap cleanup INT cleanup export CFLAGS="-O3 -fno-strict-aliasing -I/usr/lib/x86_64-redhat-linux6E/include -B /usr/lib/x86_64-redhat-linux6E/lib64" +# TODO: Figure out how to make this apply only to the contrib/ directory. Maybe override the yaml directory's Makefile.am after extracting it. +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119 +CFLAGS+=" -Werror -Wall -Wno-pointer-sign -Wno-sign-conversion -Wno-missing-braces -Wno-unused-value -Wno-builtin-declaration-mismatch -Wno-maybe-uninitialized" export LDFLAGS="-lc_nonshared" cd /usr/src/twemproxy autoreconf -fvi -./configure --enable-debug=log --prefix=/usr/src/twemproxy/work/usr +./configure --enable-debug=yes --prefix=/usr/src/twemproxy/work/usr make -j5 make install diff --git a/configure.ac b/configure.ac index 4242fb90..519c0928 100644 --- a/configure.ac +++ b/configure.ac @@ -199,11 +199,11 @@ AS_IF([test "x$disable_stats" = xyes], [AC_DEFINE([HAVE_STATS], [1], [Define to 1 if stats is not disabled])]) AC_MSG_RESULT($disable_stats) -# Untar the yaml-0.1.4 in contrib/ before config.status is rerun -AC_CONFIG_COMMANDS_PRE([tar xvfz contrib/yaml-0.1.4.tar.gz -C contrib]) +# Untar the yaml-0.2.5 in contrib/ before config.status is rerun +AC_CONFIG_COMMANDS_PRE([tar xvfz contrib/yaml-0.2.5.tar.gz -C contrib]) -# Call yaml-0.1.4 ./configure recursively -AC_CONFIG_SUBDIRS([contrib/yaml-0.1.4]) +# Call yaml-0.2.5 ./configure recursively +AC_CONFIG_SUBDIRS([contrib/yaml-0.2.5]) # Define Makefiles AC_CONFIG_FILES([Makefile diff --git a/contrib/Makefile.am b/contrib/Makefile.am index a5c85a3a..d90e6080 100644 --- a/contrib/Makefile.am +++ b/contrib/Makefile.am @@ -1,3 +1,3 @@ -SUBDIRS = yaml-0.1.4 +SUBDIRS = yaml-0.2.5 -EXTRA_DIST = yaml-0.1.4.tar.gz +EXTRA_DIST = yaml-0.2.5.tar.gz diff --git a/contrib/yaml-0.1.4.tar.gz b/contrib/yaml-0.1.4.tar.gz deleted file mode 100644 index 87a7b8f7..00000000 Binary files a/contrib/yaml-0.1.4.tar.gz and /dev/null differ diff --git a/contrib/yaml-0.2.5.tar.gz b/contrib/yaml-0.2.5.tar.gz new file mode 100644 index 00000000..a78e0d99 Binary files /dev/null and b/contrib/yaml-0.2.5.tar.gz differ diff --git a/contrib/yaml-0.1.4/.gitignore b/contrib/yaml-0.2.5/.gitignore similarity index 100% rename from contrib/yaml-0.1.4/.gitignore rename to contrib/yaml-0.2.5/.gitignore diff --git a/notes/c-styleguide.txt b/notes/c-styleguide.txt index 30aa62f2..85fd16cc 100644 --- a/notes/c-styleguide.txt +++ b/notes/c-styleguide.txt @@ -6,7 +6,7 @@ - Make sure that your editor does not leave space at the end of the line. - snake_case for variable, function and file names. - Use your own judgment when naming variables and functions. Be as Spartan - as possible. Eg: Using name like this_variable_is_a_temporary_counter + as possible. E.g.: Using name like this_variable_is_a_temporary_counter will usually be frowned upon. - Don’t use local variables or parameters that shadow global identifiers. GCC’s ‘-Wshadow’ option can help you to detect this problem. @@ -17,19 +17,19 @@ you cannot get away from using int and char. - Use bool for boolean variables. You have to include - Avoid using a bool as type for struct member names. Instead use unsigned - 1-bit bit field. Eg: + 1-bit bit field. E.g.: struct foo { unsigned is_bar:1; }; - Always use size_t type when dealing with sizes of objects or memory ranges. - Your code should be 64-bit and 32-bit friendly. Bear in mind problems of printing, comparisons, and structure alignment. You have to include - to get generic format specifier macros for printing. + to get generic format specifier macros for printing. - 80 column line limit. - If you have to wrap a long statement (> 80 column), put the operator at the end of the line and indent the next line at the same column as the arguments - in the previous column. Eg: + in the previous column. E.g.: while (cnt < 20 && this_variable_name_is_too_long && ep != NULL) { z = a + really + long + statement + that + needs + three + lines + @@ -43,19 +43,19 @@ param_g, param_h, param_i, param_j, param_k, param_l); - Always use braces for all conditional blocks (if, switch, for, while, do). - This holds good even for single statement conditional blocks. Eg: + This holds good even for single statement conditional blocks. E.g.: if (cond) { stmt; } - Placement of braces for non-function statement blocks - put opening brace - last on the line and closing brace first. Eg: + last on the line and closing brace first. E.g.: if (x is true) { we do y } - Placement of brace for functions - put the opening brace at the beginning of the next line and closing brace first. This is useful because several tools look for opening brace in column one to find beginning of C - functions. Eg: + functions. E.g.: int function(int x) { @@ -79,7 +79,7 @@ function(int x) .... } -- Column align switch keyword and the corresponding case/default keyword. Eg: +- Column align switch keyword and the corresponding case/default keyword. E.g.: switch (alphabet) { case 'a': case 'b': @@ -90,7 +90,7 @@ function(int x) break; } -- Forever loops are done with for, and not while. Eg: +- Forever loops are done with for, and not while. E.g.: for (;;) { stmt; } @@ -102,14 +102,14 @@ function(int x) - Do not add spaces around (inside) parenthesized expressions. s = sizeof( sizeof(*p)) ); /* bad example */ s = sizeof(sizeof(*p)); /* good example */ -- Casts should not be followed by space. Eg: +- Casts should not be followed by space. E.g.: int q = *(int *)&p - There is no need to type cast when assigning a void pointer to a non-void pointer, or vice versa. - Avoid using goto statements. However there are some exceptions to this rule when a single goto label within a function and one or more goto statements come in handy when a function exits from multiple locations and some common - work such as cleanup has to be done. Eg: + work such as cleanup has to be done. E.g.: int fun(void) { @@ -135,7 +135,7 @@ out: return result; } - When declaring pointer data, use '*' adjacent to the data name and not - adjacent to the type name. Eg: + adjacent to the type name. E.g.: int function(int *p) { @@ -192,7 +192,7 @@ out: or by the header that uses it (which causes namespace pollution), or there must be a back-door mechanism for obtaining the typedef. - The only exception for using a typedef is when you are defining a type - for a function pointer or a type for an enum. Eg: + for a function pointer or a type for an enum. E.g.: typedef void (*foo_handler_t)(int, void *); @@ -205,7 +205,7 @@ out: - Use just one variable declaration per line when variables are part of a struct. This leaves you room for a small comment on each item, explaining - its use. Declarations should also be aligned. Eg, use: + its use. Declarations should also be aligned. E.g., use: struct foo { int *foo_a; /* comment for foo_a */ @@ -222,7 +222,7 @@ out: - For variable declaration outside a struct, either collect all the declarations of the same type on a single line, or use one variable - per line if the variables purpose needs to be commented. Eg: + per line if the variables purpose needs to be commented. E.g.: char *a, *b, c; or: @@ -235,7 +235,7 @@ out: - Function definitions should start the name of the function in column one. This is useful because it makes searching for function definitions - fairly trivial. Eg: + fairly trivial. E.g.: static char * concat(char *s1, char *s2) { @@ -244,7 +244,7 @@ concat(char *s1, char *s2) - Function and variables local to a file should be static. - Separate two successive functions with one blank line. -- Include parameter names with their datypes in function declaration. Eg: +- Include parameter names with their datatypes in function declaration. E.g.: void function(int param); - Functions should be short and sweet, and do just one thing. They should @@ -293,7 +293,7 @@ void function(int param); type char * which is really the address of the second character of a string, not the first), or any possible values that would not work the way one would expect (such as, that strings containing newlines are not - guaranteed to work), be sure to say so. Eg: + guaranteed to work), be sure to say so. E.g.: /* * Try to acquire a physical address lock while a pmap is locked. If we @@ -324,7 +324,7 @@ vm_page_pa_tryrelock(pmap_t pmap, vm_paddr_t pa, vm_paddr_t *locked) - Recommend using UPPERCASE for macro names. However, sometimes using lowercase for macro names makes sense when macros masquerade as well-known - function calls. Eg, it makes sense to write the wrapper for the + function calls. E.g., it makes sense to write the wrapper for the standard free() function in lowercase to keep the readability consistent: @@ -340,10 +340,10 @@ vm_page_pa_tryrelock(pmap_t pmap, vm_paddr_t pa, vm_paddr_t *locked) - For macros encapsulating compound statements, right justify the backslashes and enclose it in do { ... } while (0) - For parameterized macros, all the parameters used in the macro body must - be surrounded by parentheses. Eg: + be surrounded by parentheses. E.g.: #define ADD_1(_x) ((_x) + 1) -- Use sizeof(varname) instead of sizeof(type) whenever possible. Eg: +- Use sizeof(varname) instead of sizeof(type) whenever possible. E.g.: char *p; p = malloc(sizeof(*p)); /* good example */ p = malloc(sizeof(char)); /* bad example */ @@ -369,7 +369,7 @@ vm_page_pa_tryrelock(pmap_t pmap, vm_paddr_t pa, vm_paddr_t *locked) - Every header file in the source code must have preprocessor conditional to prevent the header file from being scanned multiple times and avoiding mutual dependency cycles. Alternatively you can use #pragma once directive, - as it avoids name clashes and increases the compile speed. Eg, for a + as it avoids name clashes and increases the compile speed. E.g., for a header file named foo.h, the entire contents of the header file must be between the guard macros as follows: @@ -398,7 +398,7 @@ Or, - Conditional compilation: when supporting configuration options already known when building your program we prefer using if (... ) over conditional compilation, as in the former case the compiler is able to perform more - extensive checking of all possible code paths. Eg, use: + extensive checking of all possible code paths. E.g., use: if (HAS_FOO) ... diff --git a/notes/debug.txt b/notes/debug.txt index 282bfc5c..a746baf1 100644 --- a/notes/debug.txt +++ b/notes/debug.txt @@ -1,7 +1,7 @@ - strace strace -o strace.txt -ttT -s 1024 -p `pgrep nutcracker` -- libyaml (yaml-0.1.4) +- libyaml (yaml-0.2.5) - yaml tokens: diff --git a/notes/recommendation.md b/notes/recommendation.md index d60bcda0..f82d473e 100644 --- a/notes/recommendation.md +++ b/notes/recommendation.md @@ -158,9 +158,3 @@ You can also graph the timestamp at which any given server was ejected by graphi By design, twemproxy multiplexes several client connections over few server connections. It is important to note that **"read my last write"** constraint doesn't necessarily hold true when twemproxy is configured with `server_connections: > 1`. To illustrate this, consider a scenario where twemproxy is configured with `server_connections: 2`. If a client makes pipelined requests with the first request in pipeline being `set foo 0 0 3\r\nbar\r\n` (write) and the second request being `get foo\r\n` (read), the expectation is that the read of key `foo` would return the value `bar`. However, with configuration of two server connections it is possible that write and read request are sent on different server connections which would mean that their completion could race with one another. In summary, if the client expects "read my last write" constraint, you either configure twemproxy to use `server_connections:1` or use clients that only make synchronous requests to twemproxy. - -## twemproxy and python-memcached - -The implementation of delete command in [python-memcached](https://github.com/linsomniac/python-memcached) conflicts with the one in twemproxy. See [issue 283](https://github.com/twitter/twemproxy/pull/283) for details. The workaround for this issue is to call `delete_multi` in python-memcached as follows: - - mc.delete_multi([key1, key2, ... keyN], time=None) diff --git a/src/Makefile.am b/src/Makefile.am index 3f1f23ec..ed653918 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -7,9 +7,9 @@ endif AM_CPPFLAGS += -I $(top_srcdir)/src/hashkit AM_CPPFLAGS += -I $(top_srcdir)/src/proto AM_CPPFLAGS += -I $(top_srcdir)/src/event -AM_CPPFLAGS += -I $(top_srcdir)/contrib/yaml-0.1.4/include +AM_CPPFLAGS += -I $(top_srcdir)/contrib/yaml-0.2.5/include -AM_CFLAGS = +AM_CFLAGS = # about -fno-strict-aliasing: https://github.com/twitter/twemproxy/issues/276 AM_CFLAGS += -fno-strict-aliasing AM_CFLAGS += -Wall -Wshadow @@ -19,6 +19,7 @@ AM_CFLAGS += -Wunused-function -Wunused-variable -Wunused-value AM_CFLAGS += -Wno-unused-parameter -Wno-unused-value AM_CFLAGS += -Wconversion -Wsign-compare AM_CFLAGS += -Wstrict-prototypes -Wmissing-prototypes -Wredundant-decls -Wmissing-declarations +AM_CFLAGS += -Wno-format-zero-length AM_LDFLAGS = AM_LDFLAGS += -lm -lpthread -rdynamic @@ -58,7 +59,7 @@ nutcracker_SOURCES = \ nutcracker_LDADD = $(top_builddir)/src/hashkit/libhashkit.a nutcracker_LDADD += $(top_builddir)/src/proto/libproto.a nutcracker_LDADD += $(top_builddir)/src/event/libevent.a -nutcracker_LDADD += $(top_builddir)/contrib/yaml-0.1.4/src/.libs/libyaml.a +nutcracker_LDADD += $(top_builddir)/contrib/yaml-0.2.5/src/.libs/libyaml.a TESTS = test_all bin_PROGRAMS = test_all @@ -87,4 +88,4 @@ test_all_SOURCES = test_all.c \ test_all_LDADD = $(top_builddir)/src/hashkit/libhashkit.a test_all_LDADD += $(top_builddir)/src/proto/libproto.a test_all_LDADD += $(top_builddir)/src/event/libevent.a -test_all_LDADD += $(top_builddir)/contrib/yaml-0.1.4/src/.libs/libyaml.a +test_all_LDADD += $(top_builddir)/contrib/yaml-0.2.5/src/.libs/libyaml.a diff --git a/src/hashkit/nc_fnv.c b/src/hashkit/nc_fnv.c index 7ea3c450..3fcf5ef5 100644 --- a/src/hashkit/nc_fnv.c +++ b/src/hashkit/nc_fnv.c @@ -17,10 +17,10 @@ #include -static uint64_t FNV_64_INIT = UINT64_C(0xcbf29ce484222325); -static uint64_t FNV_64_PRIME = UINT64_C(0x100000001b3); -static uint32_t FNV_32_INIT = 2166136261UL; -static uint32_t FNV_32_PRIME = 16777619; +static const uint64_t FNV_64_INIT = UINT64_C(0xcbf29ce484222325); +static const uint64_t FNV_64_PRIME = UINT64_C(0x100000001b3); +static const uint32_t FNV_32_INIT = 2166136261UL; +static const uint32_t FNV_32_PRIME = 16777619; uint32_t hash_fnv1_64(const char *key, size_t key_length) diff --git a/src/hashkit/nc_hashkit.h b/src/hashkit/nc_hashkit.h index a38e61b2..a39a7ee7 100644 --- a/src/hashkit/nc_hashkit.h +++ b/src/hashkit/nc_hashkit.h @@ -69,11 +69,11 @@ uint32_t hash_jenkins(const char *key, size_t length); uint32_t hash_murmur(const char *key, size_t length); rstatus_t ketama_update(struct server_pool *pool); -uint32_t ketama_dispatch(struct continuum *continuum, uint32_t ncontinuum, uint32_t hash); +uint32_t ketama_dispatch(const struct continuum *continuum, uint32_t ncontinuum, uint32_t hash); rstatus_t modula_update(struct server_pool *pool); -uint32_t modula_dispatch(struct continuum *continuum, uint32_t ncontinuum, uint32_t hash); +uint32_t modula_dispatch(const struct continuum *continuum, uint32_t ncontinuum, uint32_t hash); rstatus_t random_update(struct server_pool *pool); -uint32_t random_dispatch(struct continuum *continuum, uint32_t ncontinuum, uint32_t hash); +uint32_t random_dispatch(const struct continuum *continuum, uint32_t ncontinuum, uint32_t hash); uint32_t ketama_hash(const char *key, size_t key_length, uint32_t alignment); static inline bool should_keep_server_in_pool(struct server_pool *pool, struct server *server) { diff --git a/src/hashkit/nc_ketama.c b/src/hashkit/nc_ketama.c index 57dfaf70..9ae960c7 100644 --- a/src/hashkit/nc_ketama.c +++ b/src/hashkit/nc_ketama.c @@ -209,9 +209,9 @@ ketama_update(struct server_pool *pool) } uint32_t -ketama_dispatch(struct continuum *continuum, uint32_t ncontinuum, uint32_t hash) +ketama_dispatch(const struct continuum *continuum, uint32_t ncontinuum, uint32_t hash) { - struct continuum *begin, *end, *left, *right, *middle; + const struct continuum *begin, *end, *left, *right, *middle; ASSERT(continuum != NULL); ASSERT(ncontinuum != 0); diff --git a/src/hashkit/nc_md5.c b/src/hashkit/nc_md5.c index 94faa169..7bd73a8b 100644 --- a/src/hashkit/nc_md5.c +++ b/src/hashkit/nc_md5.c @@ -85,10 +85,10 @@ typedef struct { * This processes one or more 64-byte data blocks, but does NOT update * the bit counters. There are no alignment requirements. */ -static void * -body(MD5_CTX *ctx, void *data, unsigned long size) +static const void * +body(MD5_CTX *ctx, const void *data, unsigned long size) { - unsigned char *ptr; + const unsigned char *ptr; MD5_u32plus a, b, c, d; MD5_u32plus saved_a, saved_b, saved_c, saved_d; @@ -206,7 +206,7 @@ MD5_Init(MD5_CTX *ctx) } void -MD5_Update(MD5_CTX *ctx, void *data, unsigned long size) +MD5_Update(MD5_CTX *ctx, const void *data, unsigned long size) { MD5_u32plus saved_lo; unsigned long used, free; @@ -228,7 +228,7 @@ MD5_Update(MD5_CTX *ctx, void *data, unsigned long size) } memcpy(&ctx->buffer[used], data, free); - data = (unsigned char *)data + free; + data = (const unsigned char *)data + free; size -= free; body(ctx, ctx->buffer, 64); } @@ -298,7 +298,7 @@ MD5_Final(unsigned char *result, MD5_CTX *ctx) * result must be == 16 */ void -md5_signature(unsigned char *key, unsigned long length, unsigned char *result) +md5_signature(const unsigned char *key, unsigned long length, unsigned char *result) { MD5_CTX my_md5; @@ -312,7 +312,7 @@ hash_md5(const char *key, size_t key_length) { unsigned char results[16]; - md5_signature((unsigned char*)key, (unsigned long)key_length, results); + md5_signature((const unsigned char*)key, (unsigned long)key_length, results); return ((uint32_t) (results[3] & 0xFF) << 24) | ((uint32_t) (results[2] & 0xFF) << 16) | diff --git a/src/hashkit/nc_modula.c b/src/hashkit/nc_modula.c index 1bed98fc..65300e6a 100644 --- a/src/hashkit/nc_modula.c +++ b/src/hashkit/nc_modula.c @@ -131,9 +131,9 @@ modula_update(struct server_pool *pool) } uint32_t -modula_dispatch(struct continuum *continuum, uint32_t ncontinuum, uint32_t hash) +modula_dispatch(const struct continuum *continuum, uint32_t ncontinuum, uint32_t hash) { - struct continuum *c; + const struct continuum *c; ASSERT(continuum != NULL); ASSERT(ncontinuum != 0); diff --git a/src/hashkit/nc_random.c b/src/hashkit/nc_random.c index b6174246..ec28efc5 100644 --- a/src/hashkit/nc_random.c +++ b/src/hashkit/nc_random.c @@ -124,9 +124,9 @@ random_update(struct server_pool *pool) } uint32_t -random_dispatch(struct continuum *continuum, uint32_t ncontinuum, uint32_t hash) +random_dispatch(const struct continuum *continuum, uint32_t ncontinuum, uint32_t hash) { - struct continuum *c; + const struct continuum *c; ASSERT(continuum != NULL); ASSERT(ncontinuum != 0); diff --git a/src/nc.c b/src/nc.c index 7f0508a8..0d606bad 100644 --- a/src/nc.c +++ b/src/nc.c @@ -51,7 +51,7 @@ static int test_conf; static int daemonize; static int describe_stats; -static struct option long_options[] = { +static const struct option long_options[] = { { "help", no_argument, NULL, 'h' }, { "version", no_argument, NULL, 'V' }, { "test-conf", no_argument, NULL, 't' }, @@ -68,7 +68,7 @@ static struct option long_options[] = { { NULL, 0, NULL, 0 } }; -static char short_options[] = "hVtdDv:o:c:s:i:a:p:m:"; +static const char short_options[] = "hVtdDv:o:c:s:i:a:p:m:"; static rstatus_t nc_daemonize(int dump_core) @@ -173,7 +173,7 @@ nc_daemonize(int dump_core) } static void -nc_print_run(struct instance *nci) +nc_print_run(const struct instance *nci) { int status; struct utsname name; @@ -446,7 +446,7 @@ nc_get_options(int argc, char **argv, struct instance *nci) * returns false */ static bool -nc_test_conf(struct instance *nci) +nc_test_conf(const struct instance *nci) { struct conf *cf; diff --git a/src/nc_array.c b/src/nc_array.c index c42f4532..efc8c08c 100644 --- a/src/nc_array.c +++ b/src/nc_array.c @@ -135,14 +135,6 @@ array_pop(struct array *a) return elem; } -void * -array_top(const struct array *a) -{ - ASSERT(a->nelem != 0); - - return array_get(a, a->nelem - 1); -} - void * array_get(const struct array *a, uint32_t idx) { @@ -156,6 +148,14 @@ array_get(const struct array *a, uint32_t idx) return elem; } +void * +array_top(const struct array *a) +{ + ASSERT(a->nelem != 0); + + return array_get(a, a->nelem - 1); +} + void array_swap(struct array *a, struct array *b) { @@ -183,7 +183,7 @@ array_sort(struct array *a, array_compare_t compare) * success. On failure short-circuits and returns the error status. */ rstatus_t -array_each(struct array *a, array_each_t func, void *data) +array_each(const struct array *a, array_each_t func, void *data) { uint32_t i, nelem; diff --git a/src/nc_array.h b/src/nc_array.h index aff85093..7fafc5ed 100644 --- a/src/nc_array.h +++ b/src/nc_array.h @@ -83,10 +83,10 @@ void array_deinit(struct array *a); uint32_t array_idx(const struct array *a, const void *elem); void *array_push(struct array *a); void *array_pop(struct array *a); -void *array_top(const struct array *a); void *array_get(const struct array *a, uint32_t idx); +void *array_top(const struct array *a); void array_swap(struct array *a, struct array *b); void array_sort(struct array *a, array_compare_t compare); -rstatus_t array_each(struct array *a, array_each_t func, void *data); +rstatus_t array_each(const struct array *a, array_each_t func, void *data); #endif diff --git a/src/nc_client.c b/src/nc_client.c index 291b0ff0..68b3f394 100644 --- a/src/nc_client.c +++ b/src/nc_client.c @@ -66,7 +66,7 @@ client_unref(struct conn *conn) } bool -client_active(struct conn *conn) +client_active(const struct conn *conn) { ASSERT(conn->client && !conn->proxy); diff --git a/src/nc_client.h b/src/nc_client.h index cde4ce6e..9a7b0474 100644 --- a/src/nc_client.h +++ b/src/nc_client.h @@ -20,7 +20,7 @@ #include -bool client_active(struct conn *conn); +bool client_active(const struct conn *conn); void client_ref(struct conn *conn, void *owner); void client_unref(struct conn *conn); void client_close(struct context *ctx, struct conn *conn); diff --git a/src/nc_conf.c b/src/nc_conf.c index 4b561de5..379ef2cc 100644 --- a/src/nc_conf.c +++ b/src/nc_conf.c @@ -21,27 +21,27 @@ #include #define DEFINE_ACTION(_hash, _name) string(#_name), -static struct string hash_strings[] = { +static const struct string hash_strings[] = { HASH_CODEC( DEFINE_ACTION ) null_string }; #undef DEFINE_ACTION #define DEFINE_ACTION(_hash, _name) hash_##_name, -static hash_t hash_algos[] = { +static const hash_t hash_algos[] = { HASH_CODEC( DEFINE_ACTION ) NULL }; #undef DEFINE_ACTION #define DEFINE_ACTION(_dist, _name) string(#_name), -static struct string dist_strings[] = { +static const struct string dist_strings[] = { DIST_CODEC( DEFINE_ACTION ) null_string }; #undef DEFINE_ACTION -static struct command conf_commands[] = { +static const struct command conf_commands[] = { { string("listen"), conf_set_listen, offsetof(struct conf_pool, listen) }, @@ -121,6 +121,9 @@ static struct command conf_commands[] = { null_command }; +static const struct string true_str = string("true"); +static const struct string false_str = string("false"); + static void conf_server_init(struct conf_server *cs) { @@ -188,7 +191,7 @@ conf_server_each_transform(void *elem, void *data) } static rstatus_t -conf_pool_init(struct conf_pool *cp, struct string *name) +conf_pool_init(struct conf_pool *cp, const struct string *name) { rstatus_t status; @@ -349,7 +352,7 @@ conf_pool_each_transform(void *elem, void *data) } static void -conf_dump(struct conf *cf) +conf_dump(const struct conf *cf) { uint32_t i, j, npool, nserver; struct conf_pool *cp; @@ -445,7 +448,7 @@ conf_rewrite(struct context *ctx) struct conf *cf; struct conf_pool *cp; struct conf_server *cs; - struct string true_str, false_str, bool_str; + struct string bool_str; FILE *fh; char temp_conf_file[256]; @@ -468,9 +471,6 @@ conf_rewrite(struct context *ctx) return; } - string_set_text(&true_str, "true"); - string_set_text(&false_str, "false"); - for (i = 0; i < npool; i++) { cp = array_get_known_type(&cf->pool, i, struct conf_pool); @@ -698,7 +698,7 @@ conf_pop_scalar(struct conf *cf) static rstatus_t conf_handler(struct conf *cf, void *data) { - struct command *cmd; + const struct command *cmd; struct string *key, *value; uint32_t narg; @@ -716,7 +716,7 @@ conf_handler(struct conf *cf, void *data) value->len, value->data); for (cmd = conf_commands; cmd->name.len != 0; cmd++) { - char *rv; + const char *rv; if (string_compare(key, &cmd->name) != 0) { continue; @@ -951,7 +951,7 @@ conf_parse(struct conf *cf) } static struct conf * -conf_open(char *filename) +conf_open(const char *filename) { rstatus_t status; struct conf *cf; @@ -1341,14 +1341,6 @@ conf_pre_validate(struct conf *cf) return NC_OK; } -static int -conf_server_pname_cmp(const void *t1, const void *t2) -{ - const struct conf_server *s1 = t1, *s2 = t2; - - return string_compare(&s1->pname, &s2->pname); -} - static int conf_server_name_cmp(const void *t1, const void *t2) { @@ -1616,7 +1608,7 @@ conf_post_validate(struct conf *cf) } struct conf * -conf_create(char *filename) +conf_create(const char *filename) { rstatus_t status; struct conf *cf; @@ -1676,12 +1668,13 @@ conf_destroy(struct conf *cf) nc_free(cf); } -char * -conf_set_string(struct conf *cf, struct command *cmd, void *conf) +const char * +conf_set_string(struct conf *cf, const struct command *cmd, void *conf) { rstatus_t status; uint8_t *p; - struct string *field, *value; + struct string *field; + const struct string *value; p = conf; field = (struct string *)(p + cmd->offset); @@ -1700,8 +1693,8 @@ conf_set_string(struct conf *cf, struct command *cmd, void *conf) return CONF_OK; } -char * -conf_set_listen(struct conf *cf, struct command *cmd, void *conf) +const char * +conf_set_listen(struct conf *cf, const struct command *cmd, void *conf) { rstatus_t status; struct string *value; @@ -1725,8 +1718,6 @@ conf_set_listen(struct conf *cf, struct command *cmd, void *conf) if (value->data[0] == '/') { uint8_t *q, *start, *perm; - uint32_t permlen; - /* parse "socket_path permissions" from the end */ p = value->data + value->len -1; @@ -1739,7 +1730,6 @@ conf_set_listen(struct conf *cf, struct command *cmd, void *conf) field->perm = (mode_t)0; } else { perm = q + 1; - permlen = (uint32_t)(p - perm + 1); p = q - 1; name = start; @@ -1792,8 +1782,8 @@ conf_set_listen(struct conf *cf, struct command *cmd, void *conf) return CONF_OK; } -char * -conf_add_server(struct conf *cf, struct command *cmd, void *conf) +const char * +conf_add_server(struct conf *cf, const struct command *cmd, void *conf) { rstatus_t status; struct array *a; @@ -1802,7 +1792,7 @@ conf_add_server(struct conf *cf, struct command *cmd, void *conf) uint8_t *p, *q, *start; uint8_t *pname, *addr, *port, *weight, *name; uint32_t k, delimlen, pnamelen, addrlen, portlen, weightlen, namelen; - char delim[] = " ::"; + const char *const delim = " ::"; p = conf; a = (struct array *)(p + cmd->offset); @@ -1932,12 +1922,12 @@ conf_add_server(struct conf *cf, struct command *cmd, void *conf) return CONF_OK; } -char * -conf_set_num(struct conf *cf, struct command *cmd, void *conf) +const char * +conf_set_num(struct conf *cf, const struct command *cmd, void *conf) { uint8_t *p; int num, *np; - struct string *value; + const struct string *value; p = conf; np = (int *)(p + cmd->offset); @@ -1958,12 +1948,12 @@ conf_set_num(struct conf *cf, struct command *cmd, void *conf) return CONF_OK; } -char * -conf_set_bool(struct conf *cf, struct command *cmd, void *conf) +const char * +conf_set_bool(struct conf *cf, const struct command *cmd, void *conf) { uint8_t *p; int *bp; - struct string *value, true_str, false_str; + const struct string *value; p = conf; bp = (int *)(p + cmd->offset); @@ -1973,8 +1963,6 @@ conf_set_bool(struct conf *cf, struct command *cmd, void *conf) } value = array_top(&cf->arg); - string_set_text(&true_str, "true"); - string_set_text(&false_str, "false"); if (string_compare(value, &true_str) == 0) { *bp = 1; @@ -1987,12 +1975,12 @@ conf_set_bool(struct conf *cf, struct command *cmd, void *conf) return CONF_OK; } -char * -conf_set_hash(struct conf *cf, struct command *cmd, void *conf) +const char * +conf_set_hash(struct conf *cf, const struct command *cmd, void *conf) { uint8_t *p; hash_type_t *hp; - struct string *value, *hash; + const struct string *value, *hash; p = conf; hp = (hash_type_t *)(p + cmd->offset); @@ -2016,12 +2004,12 @@ conf_set_hash(struct conf *cf, struct command *cmd, void *conf) return "is not a valid hash"; } -char * -conf_set_distribution(struct conf *cf, struct command *cmd, void *conf) +const char * +conf_set_distribution(struct conf *cf, const struct command *cmd, void *conf) { uint8_t *p; dist_type_t *dp; - struct string *value, *dist; + const struct string *value, *dist; p = conf; dp = (dist_type_t *)(p + cmd->offset); @@ -2045,12 +2033,13 @@ conf_set_distribution(struct conf *cf, struct command *cmd, void *conf) return "is not a valid distribution"; } -char * -conf_set_hashtag(struct conf *cf, struct command *cmd, void *conf) +const char * +conf_set_hashtag(struct conf *cf, const struct command *cmd, void *conf) { rstatus_t status; uint8_t *p; - struct string *field, *value; + struct string *field; + const struct string *value; p = conf; field = (struct string *)(p + cmd->offset); diff --git a/src/nc_conf.h b/src/nc_conf.h index 63560ecf..731c97df 100644 --- a/src/nc_conf.h +++ b/src/nc_conf.h @@ -103,7 +103,7 @@ struct conf_pool { }; struct conf { - char *fname; /* file name (ref in argv[]) */ + const char *fname; /* file name (ref in argv[]) */ FILE *fh; /* file handle */ struct array arg; /* string[] (parsed {key, value} pairs) */ struct array pool; /* conf_pool[] (parsed pools) */ @@ -122,25 +122,25 @@ struct conf { struct command { struct string name; - char *(*set)(struct conf *cf, struct command *cmd, void *data); + const char *(*set)(struct conf *cf, const struct command *cmd, void *data); int offset; }; #define null_command { null_string, NULL, 0 } -char *conf_set_string(struct conf *cf, struct command *cmd, void *conf); -char *conf_set_listen(struct conf *cf, struct command *cmd, void *conf); -char *conf_add_server(struct conf *cf, struct command *cmd, void *conf); -char *conf_set_num(struct conf *cf, struct command *cmd, void *conf); -char *conf_set_bool(struct conf *cf, struct command *cmd, void *conf); -char *conf_set_hash(struct conf *cf, struct command *cmd, void *conf); -char *conf_set_distribution(struct conf *cf, struct command *cmd, void *conf); -char *conf_set_hashtag(struct conf *cf, struct command *cmd, void *conf); +const char *conf_set_string(struct conf *cf, const struct command *cmd, void *conf); +const char *conf_set_listen(struct conf *cf, const struct command *cmd, void *conf); +const char *conf_add_server(struct conf *cf, const struct command *cmd, void *conf); +const char *conf_set_num(struct conf *cf, const struct command *cmd, void *conf); +const char *conf_set_bool(struct conf *cf, const struct command *cmd, void *conf); +const char *conf_set_hash(struct conf *cf, const struct command *cmd, void *conf); +const char *conf_set_distribution(struct conf *cf, const struct command *cmd, void *conf); +const char *conf_set_hashtag(struct conf *cf, const struct command *cmd, void *conf); rstatus_t conf_server_each_transform(void *elem, void *data); rstatus_t conf_pool_each_transform(void *elem, void *data); -struct conf *conf_create(char *filename); +struct conf *conf_create(const char *filename); void conf_destroy(struct conf *cf); void conf_rewrite(struct context *ctx); diff --git a/src/nc_connection.c b/src/nc_connection.c index 89e23087..830010a1 100644 --- a/src/nc_connection.c +++ b/src/nc_connection.c @@ -92,7 +92,7 @@ static uint32_t ncurr_cconn; /* current # client connections */ * Return the context associated with this connection. */ struct context * -conn_to_ctx(struct conn *conn) +conn_to_ctx(const struct conn *conn) { struct server_pool *pool; @@ -261,9 +261,8 @@ conn_get(void *owner, bool client, bool redis) } struct conn * -conn_get_proxy(void *owner) +conn_get_proxy(struct server_pool *pool) { - struct server_pool *pool = owner; struct conn *conn; conn = _conn_get(); @@ -295,7 +294,7 @@ conn_get_proxy(void *owner) conn->enqueue_outq = NULL; conn->dequeue_outq = NULL; - conn->ref(conn, owner); + conn->ref(conn, pool); log_debug(LOG_VVERB, "get conn %p proxy %d", conn, conn->proxy); @@ -329,7 +328,7 @@ conn_put(struct conn *conn) void conn_init(void) { - log_debug(LOG_DEBUG, "conn size %ld", sizeof(struct conn)); + log_debug(LOG_DEBUG, "conn size %d", (int)sizeof(struct conn)); nfree_connq = 0; TAILQ_INIT(&free_connq); } @@ -399,7 +398,7 @@ conn_recv(struct conn *conn, void *buf, size_t size) } ssize_t -conn_sendv(struct conn *conn, struct array *sendv, size_t nsend) +conn_sendv(struct conn *conn, const struct array *sendv, size_t nsend) { ssize_t n; @@ -470,7 +469,7 @@ conn_ncurr_cconn(void) * authentication, otherwise return false */ bool -conn_authenticated(struct conn *conn) +conn_authenticated(const struct conn *conn) { struct server_pool *pool; diff --git a/src/nc_connection.h b/src/nc_connection.h index a0f4fe07..e2e07e3f 100644 --- a/src/nc_connection.h +++ b/src/nc_connection.h @@ -37,7 +37,7 @@ typedef struct msg* (*conn_send_next_t)(struct context *, struct conn *); typedef void (*conn_send_done_t)(struct context *, struct conn *, struct msg *); typedef void (*conn_close_t)(struct context *, struct conn *); -typedef bool (*conn_active_t)(struct conn *); +typedef bool (*conn_active_t)(const struct conn *); typedef void (*conn_ref_t)(struct conn *, void *); typedef void (*conn_unref_t)(struct conn *); @@ -106,18 +106,18 @@ struct conn { TAILQ_HEAD(conn_tqh, conn); -struct context *conn_to_ctx(struct conn *conn); +struct context *conn_to_ctx(const struct conn *conn); struct conn *conn_get(void *owner, bool client, bool redis); -struct conn *conn_get_proxy(void *owner); +struct conn *conn_get_proxy(struct server_pool *pool); void conn_put(struct conn *conn); ssize_t conn_recv(struct conn *conn, void *buf, size_t size); -ssize_t conn_sendv(struct conn *conn, struct array *sendv, size_t nsend); +ssize_t conn_sendv(struct conn *conn, const struct array *sendv, size_t nsend); void conn_init(void); void conn_deinit(void); uint32_t conn_ncurr_conn(void); uint64_t conn_ntotal_conn(void); uint32_t conn_ncurr_cconn(void); -bool conn_authenticated(struct conn *conn); +bool conn_authenticated(const struct conn *conn); rstatus_t event_add_out_with_conn(struct context *ctx, struct conn *conn, struct msg *msg); #endif diff --git a/src/nc_core.c b/src/nc_core.c index aadb71fb..67f21210 100644 --- a/src/nc_core.c +++ b/src/nc_core.c @@ -34,21 +34,6 @@ core_failed_servers_init(struct context *ctx) } } -static void -core_failed_servers_deinit(struct context *ctx) -{ - uint32_t i, n, nsize; - - for (i = 0; i < 2; i++) { - nsize = array_n(&(ctx->failed_servers[i])); - for (n = 0; n < nsize; n++) { - /* This is buggy but core_failed_servers_deinit isn't even referenced - failed_servers is 2 arrays */ - array_pop(&(ctx->failed_servers[n])); - } - array_deinit(&(ctx->failed_servers[n])); - } -} - static rstatus_t core_calc_connections(struct context *ctx) { @@ -252,7 +237,8 @@ static void core_close(struct context *ctx, struct conn *conn) { rstatus_t status; - char type, *addrstr; + char type; + const char *addrstr; ASSERT(conn->sd > 0); diff --git a/src/nc_core.h b/src/nc_core.h index ef834722..3e919efe 100644 --- a/src/nc_core.h +++ b/src/nc_core.h @@ -141,15 +141,15 @@ struct context { struct instance { struct context *ctx; /* active context */ int log_level; /* log level */ - char *log_filename; /* log filename */ - char *conf_filename; /* configuration filename */ + const char *log_filename; /* log filename */ + const char *conf_filename; /* configuration filename */ uint16_t stats_port; /* stats monitoring port */ int stats_interval; /* stats aggregation interval */ - char *stats_addr; /* stats monitoring addr */ + const char *stats_addr; /* stats monitoring addr */ char hostname[NC_MAXHOSTNAMELEN]; /* hostname */ size_t mbuf_chunk_size; /* mbuf chunk size */ pid_t pid; /* process id */ - char *pid_filename; /* pid filename */ + const char *pid_filename; /* pid filename */ unsigned pidfile:1; /* pid file created? */ }; diff --git a/src/nc_log.c b/src/nc_log.c index 3d451ffc..7beca678 100644 --- a/src/nc_log.c +++ b/src/nc_log.c @@ -27,7 +27,7 @@ static struct logger logger; int -log_init(int level, char *name) +log_init(int level, const char *name) { struct logger *l = &logger; @@ -202,7 +202,7 @@ _log_stderr(const char *fmt, ...) * See -C option in man hexdump */ void -_log_hexdump(const char *file, int line, char *data, int datalen, +_log_hexdump(const char *file, int line, const char *data, int datalen, const char *fmt, ...) { struct logger *l = &logger; @@ -221,7 +221,7 @@ _log_hexdump(const char *file, int line, char *data, int datalen, size = 8 * LOG_MAX_LEN; /* size of output buffer */ while (datalen != 0 && (len < size - 1)) { - char *save, *str; + const char *save, *str; unsigned char c; int savelen; diff --git a/src/nc_log.h b/src/nc_log.h index 80f82196..df55d307 100644 --- a/src/nc_log.h +++ b/src/nc_log.h @@ -18,11 +18,13 @@ #ifndef _NC_LOG_H_ #define _NC_LOG_H_ +#include + struct logger { - char *name; /* log file name */ - int level; /* log level */ - int fd; /* log file descriptor */ - int nerror; /* # log error */ + const char *name; /* log file name */ + int level; /* log level */ + int fd; /* log file descriptor */ + int nerror; /* # log error */ }; #define LOG_EMERG 0 /* system in unusable */ @@ -115,18 +117,7 @@ struct logger { } while (0) -#ifdef __GNUC__ -# define NC_GCC_VERSION (__GNUC__ * 1000 + __GNUC_MINOR__) -#else -# define NC_GCC_VERSION 0 -#endif -#if NC_GCC_VERSION >= 2007 -#define NC_ATTRIBUTE_FORMAT(type, idx, first) __attribute__ ((format(type, idx, first))) -#else -#define NC_ATTRIBUTE_FORMAT(type, idx, first) -#endif - -int log_init(int level, char *filename); +int log_init(int level, const char *filename); void log_deinit(void); void log_level_up(void); void log_level_down(void); @@ -138,6 +129,6 @@ void _log(const char *file, int line, int panic, const char *fmt, ...) NC_ATTRIB void _log_stderr(const char *fmt, ...) NC_ATTRIBUTE_FORMAT(printf, 1, 2); void _log_safe(const char *fmt, ...) NC_ATTRIBUTE_FORMAT(printf, 1, 2); void _log_stderr_safe(const char *fmt, ...) NC_ATTRIBUTE_FORMAT(printf, 1, 2); -void _log_hexdump(const char *file, int line, char *data, int datalen, const char *fmt, ...); +void _log_hexdump(const char *file, int line, const char *data, int datalen, const char *fmt, ...); #endif diff --git a/src/nc_mbuf.c b/src/nc_mbuf.c index 2ef1c3d8..3a10b71b 100644 --- a/src/nc_mbuf.c +++ b/src/nc_mbuf.c @@ -106,7 +106,7 @@ mbuf_free(struct mbuf *mbuf) { uint8_t *buf; - log_debug(LOG_VVERB, "put mbuf %p len %ld", mbuf, mbuf->last - mbuf->pos); + log_debug(LOG_VVERB, "put mbuf %p len %d", mbuf, (int)(mbuf->last - mbuf->pos)); ASSERT(STAILQ_NEXT(mbuf, next) == NULL); ASSERT(mbuf->magic == MBUF_MAGIC); @@ -118,7 +118,7 @@ mbuf_free(struct mbuf *mbuf) void mbuf_put(struct mbuf *mbuf) { - log_debug(LOG_VVERB, "put mbuf %p len %ld", mbuf, mbuf->last - mbuf->pos); + log_debug(LOG_VVERB, "put mbuf %p len %d", mbuf, (int)(mbuf->last - mbuf->pos)); ASSERT(STAILQ_NEXT(mbuf, next) == NULL); ASSERT(mbuf->magic == MBUF_MAGIC); @@ -143,7 +143,7 @@ mbuf_rewind(struct mbuf *mbuf) * 2^32 bytes (4G). */ uint32_t -mbuf_length(struct mbuf *mbuf) +mbuf_length(const struct mbuf *mbuf) { ASSERT(mbuf->last >= mbuf->pos); @@ -155,7 +155,7 @@ mbuf_length(struct mbuf *mbuf) * contain more than 2^32 bytes (4G). */ uint32_t -mbuf_size(struct mbuf *mbuf) +mbuf_size(const struct mbuf *mbuf) { ASSERT(mbuf->end >= mbuf->last); @@ -179,7 +179,8 @@ void mbuf_insert(struct mhdr *mhdr, struct mbuf *mbuf) { STAILQ_INSERT_TAIL(mhdr, mbuf, next); - log_debug(LOG_VVERB, "insert mbuf %p len %ld", mbuf, mbuf->last - mbuf->pos); + log_debug(LOG_VVERB, "insert mbuf %p len %d", mbuf, + (int)(mbuf->last - mbuf->pos)); } /* @@ -188,7 +189,8 @@ mbuf_insert(struct mhdr *mhdr, struct mbuf *mbuf) void mbuf_remove(struct mhdr *mhdr, struct mbuf *mbuf) { - log_debug(LOG_VVERB, "remove mbuf %p len %ld", mbuf, mbuf->last - mbuf->pos); + log_debug(LOG_VVERB, "remove mbuf %p len %d", mbuf, + (int)(mbuf->last - mbuf->pos)); STAILQ_REMOVE(mhdr, mbuf, mbuf, next); STAILQ_NEXT(mbuf, next) = NULL; @@ -201,7 +203,7 @@ mbuf_remove(struct mhdr *mhdr, struct mbuf *mbuf) * enough space for n bytes. */ void -mbuf_copy(struct mbuf *mbuf, uint8_t *pos, size_t n) +mbuf_copy(struct mbuf *mbuf, const uint8_t *pos, size_t n) { if (n == 0) { return; @@ -260,7 +262,7 @@ mbuf_split(struct mhdr *h, uint8_t *pos, mbuf_copy_t cb, void *cbarg) } void -mbuf_init(struct instance *nci) +mbuf_init(const struct instance *nci) { nfree_mbufq = 0; STAILQ_INIT(&free_mbufq); diff --git a/src/nc_mbuf.h b/src/nc_mbuf.h index 19ea7e35..b5020340 100644 --- a/src/nc_mbuf.h +++ b/src/nc_mbuf.h @@ -40,28 +40,28 @@ STAILQ_HEAD(mhdr, mbuf); #define MBUF_HSIZE sizeof(struct mbuf) static inline bool -mbuf_empty(struct mbuf *mbuf) +mbuf_empty(const struct mbuf *mbuf) { - return mbuf->pos == mbuf->last ? true : false; + return mbuf->pos == mbuf->last; } static inline bool -mbuf_full(struct mbuf *mbuf) +mbuf_full(const struct mbuf *mbuf) { - return mbuf->last == mbuf->end ? true : false; + return mbuf->last == mbuf->end; } -void mbuf_init(struct instance *nci); +void mbuf_init(const struct instance *nci); void mbuf_deinit(void); struct mbuf *mbuf_get(void); void mbuf_put(struct mbuf *mbuf); void mbuf_rewind(struct mbuf *mbuf); -uint32_t mbuf_length(struct mbuf *mbuf); -uint32_t mbuf_size(struct mbuf *mbuf); +uint32_t mbuf_length(const struct mbuf *mbuf); +uint32_t mbuf_size(const struct mbuf *mbuf); size_t mbuf_data_size(void); void mbuf_insert(struct mhdr *mhdr, struct mbuf *mbuf); void mbuf_remove(struct mhdr *mhdr, struct mbuf *mbuf); -void mbuf_copy(struct mbuf *mbuf, uint8_t *pos, size_t n); +void mbuf_copy(struct mbuf *mbuf, const uint8_t *pos, size_t n); struct mbuf *mbuf_split(struct mhdr *h, uint8_t *pos, mbuf_copy_t cb, void *cbarg); rstatus_t mbuf_read_string(struct mbuf *mbuf, char c, struct string *read_string); diff --git a/src/nc_message.c b/src/nc_message.c index d1867f3f..c6633d45 100644 --- a/src/nc_message.c +++ b/src/nc_message.c @@ -117,7 +117,7 @@ static struct rbtree tmo_rbt; /* timeout rbtree */ static struct rbnode tmo_rbs; /* timeout rbtree sentinel */ #define DEFINE_ACTION(_name) string(#_name), -static struct string msg_type_strings[] = { +static const struct string msg_type_strings[] = { MSG_TYPE_CODEC( DEFINE_ACTION ) null_string }; @@ -251,16 +251,17 @@ _msg_get(void) msg->frag_id = 0; msg->frag_seq = NULL; - // These are used for parsing redis requests and responses. + /* These are used for parsing redis requests and responses. */ msg->narg_start = NULL; msg->narg_end = NULL; msg->narg = 0; msg->rnarg = 0; msg->rlen = 0; - msg->integer = 0; // This is used for both parsing redis requests and as a counter for coalescing responses such as DEL - - // It's not necessary to initialize msg->stack - it's always done in nc_redis.c. Save a few instructions. - // msg->nested_depth = 0; // Currently only used to parse redis *responses*. This may later include redis requests. + /* + * This is used for both parsing redis responses + * and as a counter for coalescing responses such as DEL + */ + msg->integer = 0; msg->err = 0; msg->raw_bitflags = 0; @@ -314,8 +315,8 @@ msg_get_error(bool redis, err_t err) struct msg *msg; struct mbuf *mbuf; int n; - char *errstr = err ? strerror(err) : "unknown"; - char *protstr = redis ? "-ERR" : "SERVER_ERROR"; + const char *errstr = err ? strerror(err) : "unknown"; + const char *protstr = redis ? "-ERR" : "SERVER_ERROR"; msg = _msg_get(); if (msg == NULL) { @@ -378,9 +379,9 @@ msg_put(struct msg *msg) } void -msg_dump(struct msg *msg, int level) +msg_dump(const struct msg *msg, int level) { - struct mbuf *mbuf; + const struct mbuf *mbuf; if (log_loggable(level) == 0) { return; @@ -427,19 +428,20 @@ msg_deinit(void) ASSERT(nfree_msgq == 0); } -struct string * +const struct string * msg_type_string(msg_type_t type) { return &msg_type_strings[type]; } bool -msg_empty(struct msg *msg) +msg_empty(const struct msg *msg) { return msg->mlen == 0; } -/* read a line from msg's mbuf, then write to line_buf, +/* + * read a line from msg's mbuf, then write to line_buf, * if line_num is n, we will skip front n - 1 lines in the msg */ void @@ -495,7 +497,7 @@ msg_read_line(struct msg* msg, struct mbuf *line_buf, int line_num) } uint32_t -msg_backend_idx(struct msg *msg, uint8_t *key, uint32_t keylen) +msg_backend_idx(const struct msg *msg, const uint8_t *key, uint32_t keylen) { struct conn *conn = msg->owner; struct server_pool *pool = conn->owner; @@ -527,7 +529,7 @@ msg_ensure_mbuf(struct msg *msg, size_t len) * into mbuf */ rstatus_t -msg_append(struct msg *msg, uint8_t *pos, size_t n) +msg_append(struct msg *msg, const uint8_t *pos, size_t n) { struct mbuf *mbuf; @@ -551,7 +553,7 @@ msg_append(struct msg *msg, uint8_t *pos, size_t n) * into mbuf */ rstatus_t -msg_prepend(struct msg *msg, uint8_t *pos, size_t n) +msg_prepend(struct msg *msg, const uint8_t *pos, size_t n) { struct mbuf *mbuf; diff --git a/src/nc_message.h b/src/nc_message.h index d79be104..f24e6b45 100644 --- a/src/nc_message.h +++ b/src/nc_message.h @@ -248,12 +248,14 @@ typedef enum msg_type { #undef DEFINE_ACTION struct keypos { - uint8_t *start; /* key start pos */ - uint8_t *end; /* key end pos */ + uint8_t *start; /* key start pos */ + uint8_t *end; /* key end pos */ }; -// This represents a message with a list of mbufs, that can be a redis/memcache request/response/error response. -// http://www.catb.org/esr/structure-packing/ may be of use +/* + * This represents a message with a list of mbufs + * that can be a redis/memcache request/response/error response. + */ struct msg { TAILQ_ENTRY(msg) c_tqe; /* link in client q */ TAILQ_ENTRY(msg) s_tqe; /* link in server q */ @@ -326,26 +328,26 @@ void msg_tmo_delete(struct msg *msg); void msg_init(void); void msg_deinit(void); -struct string *msg_type_string(msg_type_t type); +const struct string *msg_type_string(msg_type_t type); struct msg *msg_get(struct conn *conn, bool request, bool redis); void msg_put(struct msg *msg); struct msg *msg_get_error(bool redis, err_t err); -void msg_dump(struct msg *msg, int level); -bool msg_empty(struct msg *msg); -void msg_read_line(struct msg* msg, struct mbuf *line_buf, int line_num); +void msg_dump(const struct msg *msg, int level); +bool msg_empty(const struct msg *msg); rstatus_t msg_recv(struct context *ctx, struct conn *conn); rstatus_t msg_send(struct context *ctx, struct conn *conn); uint64_t msg_gen_frag_id(void); -uint32_t msg_backend_idx(struct msg *msg, uint8_t *key, uint32_t keylen); +uint32_t msg_backend_idx(const struct msg *msg, const uint8_t *key, uint32_t keylen); struct mbuf *msg_ensure_mbuf(struct msg *msg, size_t len); -rstatus_t msg_append(struct msg *msg, uint8_t *pos, size_t n); -rstatus_t msg_prepend(struct msg *msg, uint8_t *pos, size_t n); +rstatus_t msg_append(struct msg *msg, const uint8_t *pos, size_t n); +rstatus_t msg_prepend(struct msg *msg, const uint8_t *pos, size_t n); rstatus_t msg_prepend_format(struct msg *msg, const char *fmt, ...); +void msg_read_line(struct msg* msg, struct mbuf *line_buf, int line_num); struct msg *req_get(struct conn *conn); void req_put(struct msg *msg); -bool req_done(struct conn *conn, struct msg *msg); -bool req_error(struct conn *conn, struct msg *msg); +bool req_done(const struct conn *conn, struct msg *msg); +bool req_error(const struct conn *conn, struct msg *msg); void req_server_enqueue_imsgq(struct context *ctx, struct conn *conn, struct msg *msg); void req_server_enqueue_imsgq_head(struct context *ctx, struct conn *conn, struct msg *msg); void req_server_dequeue_imsgq(struct context *ctx, struct conn *conn, struct msg *msg); diff --git a/src/nc_queue.h b/src/nc_queue.h index 5ff70cf9..98f4ec02 100644 --- a/src/nc_queue.h +++ b/src/nc_queue.h @@ -168,6 +168,13 @@ struct qm_trace { (head)->trace.lastfile = __FILE__; \ } while (0) +#define QMD_TRACE_HEAD_INIT(head) do { \ + (head)->trace.prevline = __LINE__; \ + (head)->trace.prevfile = __FILE__; \ + (head)->trace.lastline = __LINE__; \ + (head)->trace.lastfile = __FILE__; \ +} while (0) + #define QMD_TRACE_ELEM(elem) do { \ (elem)->trace.prevline = (elem)->trace.lastline; \ (elem)->trace.prevfile = (elem)->trace.lastfile; \ @@ -179,6 +186,7 @@ struct qm_trace { #define QMD_TRACE_ELEM(elem) #define QMD_TRACE_HEAD(head) +#define QMD_TRACE_HEAD_INIT(head) #define TRACEBUF #endif /* QUEUE_MACRO_TRACE */ @@ -595,7 +603,7 @@ struct { \ #define TAILQ_INIT(head) do { \ TAILQ_FIRST((head)) = NULL; \ (head)->tqh_last = &TAILQ_FIRST((head)); \ - QMD_TRACE_HEAD(head); \ + QMD_TRACE_HEAD_INIT(head); \ } while (0) #define TAILQ_INSERT_AFTER(head, listelm, elm, field) do { \ diff --git a/src/nc_rbtree.c b/src/nc_rbtree.c index 404789fe..d76ad861 100644 --- a/src/nc_rbtree.c +++ b/src/nc_rbtree.c @@ -38,7 +38,7 @@ rbtree_init(struct rbtree *tree, struct rbnode *node) } static struct rbnode * -rbtree_node_min(struct rbnode *node, struct rbnode *sentinel) +rbtree_node_min(struct rbnode *node, const struct rbnode *sentinel) { /* traverse left links */ @@ -50,10 +50,10 @@ rbtree_node_min(struct rbnode *node, struct rbnode *sentinel) } struct rbnode * -rbtree_min(struct rbtree *tree) +rbtree_min(const struct rbtree *tree) { struct rbnode *node = tree->root; - struct rbnode *sentinel = tree->sentinel; + const struct rbnode *sentinel = tree->sentinel; /* empty tree */ diff --git a/src/nc_rbtree.h b/src/nc_rbtree.h index bdf19525..4b6137ac 100644 --- a/src/nc_rbtree.h +++ b/src/nc_rbtree.h @@ -40,7 +40,7 @@ struct rbtree { void rbtree_node_init(struct rbnode *node); void rbtree_init(struct rbtree *tree, struct rbnode *node); -struct rbnode *rbtree_min(struct rbtree *tree); +struct rbnode *rbtree_min(const struct rbtree *tree); void rbtree_insert(struct rbtree *tree, struct rbnode *node); void rbtree_delete(struct rbtree *tree, struct rbnode *node); diff --git a/src/nc_request.c b/src/nc_request.c index 5180d528..446f32ff 100644 --- a/src/nc_request.c +++ b/src/nc_request.c @@ -33,14 +33,14 @@ req_get(struct conn *conn) } static void -req_log(struct msg *req) +req_log(const struct msg *req) { - struct msg *rsp; /* peer message (response) */ + const struct msg *rsp; /* peer message (response) */ int64_t req_time; /* time cost for this request */ - char *peer_str; /* peer client ip:port */ + const char *peer_str; /* peer client ip:port */ uint32_t req_len, rsp_len; /* request and response length */ - struct string *req_type; /* request type string */ - struct keypos *kpos; + const struct string *req_type; /* request type string */ + const struct keypos *kpos; if (log_loggable(LOG_INFO) == 0) { return; @@ -95,7 +95,8 @@ req_log(struct msg *req) " key0 '%.*s' peer '%s' done %d error %d", req->id, req->owner->sd, req_time / 1000, req_time % 1000, req_type->len, req_type->data, req->narg, req_len, rsp_len, - (int)(kpos->end ? kpos->end - kpos->start : 0), kpos->start, peer_str, req->done, req->error); + (int)(kpos->end ? kpos->end - kpos->start : 0), kpos->start, + peer_str, req->done, req->error); } void @@ -126,11 +127,13 @@ req_put(struct msg *msg) * A request is done, if we received response for the given request. * A request vector is done if we received responses for all its * fragments. + * + * msg->fdone is modified to cache whether this request was done. */ bool -req_done(struct conn *conn, struct msg *msg) +req_done(const struct conn *conn, struct msg *msg) { - struct msg *cmsg, *pmsg; /* current and previous message */ + struct msg *cmsg; /* current message */ uint64_t id; /* fragment id */ uint32_t nfragment; /* # fragment */ @@ -157,18 +160,18 @@ req_done(struct conn *conn, struct msg *msg) /* check all fragments of the given request vector are done */ - for (pmsg = msg, cmsg = TAILQ_PREV(msg, msg_tqh, c_tqe); + for (cmsg = TAILQ_PREV(msg, msg_tqh, c_tqe); cmsg != NULL && cmsg->frag_id == id; - pmsg = cmsg, cmsg = TAILQ_PREV(cmsg, msg_tqh, c_tqe)) { + cmsg = TAILQ_PREV(cmsg, msg_tqh, c_tqe)) { if (!cmsg->done) { return false; } } - for (pmsg = msg, cmsg = TAILQ_NEXT(msg, c_tqe); + for (cmsg = TAILQ_NEXT(msg, c_tqe); cmsg != NULL && cmsg->frag_id == id; - pmsg = cmsg, cmsg = TAILQ_NEXT(cmsg, c_tqe)) { + cmsg = TAILQ_NEXT(cmsg, c_tqe)) { if (!cmsg->done) { return false; @@ -186,16 +189,16 @@ req_done(struct conn *conn, struct msg *msg) msg->fdone = 1; nfragment = 0; - for (pmsg = msg, cmsg = TAILQ_PREV(msg, msg_tqh, c_tqe); + for (cmsg = TAILQ_PREV(msg, msg_tqh, c_tqe); cmsg != NULL && cmsg->frag_id == id; - pmsg = cmsg, cmsg = TAILQ_PREV(cmsg, msg_tqh, c_tqe)) { + cmsg = TAILQ_PREV(cmsg, msg_tqh, c_tqe)) { cmsg->fdone = 1; nfragment++; } - for (pmsg = msg, cmsg = TAILQ_NEXT(msg, c_tqe); + for (cmsg = TAILQ_NEXT(msg, c_tqe); cmsg != NULL && cmsg->frag_id == id; - pmsg = cmsg, cmsg = TAILQ_NEXT(cmsg, c_tqe)) { + cmsg = TAILQ_NEXT(cmsg, c_tqe)) { cmsg->fdone = 1; nfragment++; } @@ -220,7 +223,7 @@ req_done(struct conn *conn, struct msg *msg) * receiving response for any its fragments. */ bool -req_error(struct conn *conn, struct msg *msg) +req_error(const struct conn *conn, struct msg *msg) { struct msg *cmsg; /* current message */ uint64_t id; @@ -508,7 +511,7 @@ req_fake(struct context *ctx, struct conn *conn) } static bool -req_filter(struct context *ctx, struct conn *conn, struct msg *msg) +req_filter(struct conn *conn, struct msg *msg) { ASSERT(conn->client && !conn->proxy); @@ -593,7 +596,6 @@ req_forward(struct context *ctx, struct conn *c_conn, struct msg *msg) { rstatus_t status; struct conn *s_conn; - struct server_pool *pool; uint8_t *key; uint32_t keylen; struct keypos *kpos; @@ -605,8 +607,6 @@ req_forward(struct context *ctx, struct conn *c_conn, struct msg *msg) c_conn->enqueue_outq(ctx, c_conn, msg); } - pool = c_conn->owner; - ASSERT(array_n(msg->keys) > 0); kpos = array_get_known_type(msg->keys, 0, struct keypos); key = kpos->start; @@ -614,9 +614,18 @@ req_forward(struct context *ctx, struct conn *c_conn, struct msg *msg) s_conn = server_pool_conn(ctx, c_conn->owner, key, keylen); if (s_conn == NULL) { - /* Handle a failure to establish a new connection to a server, e.g. due to dns resolution errors. */ - /* If this is a fragmented request sent to multiple servers such as a memcache get(multiget) mark the fragment for this request to the server as done. */ - /* Normally, this would be done when the request was forwarded to the server, but due to failing to connect to the server this check is repeated here */ + /* + * Handle a failure to establish a new connection to a server, + * e.g. due to dns resolution errors. + * + * If this is a fragmented request sent to multiple servers such as + * a memcache get(multiget), + * mark the fragment for this request to the server as done. + * + * Normally, this would be done when the request was forwarded to the + * server, but due to failing to connect to the server this check is + * repeated here. + */ if (msg->frag_owner != NULL) { msg->frag_owner->nfrag_done++; } @@ -673,7 +682,7 @@ req_recv_done(struct context *ctx, struct conn *conn, struct msg *msg, /* enqueue next message (request), if any */ conn->rmsg = nmsg; - if (req_filter(ctx, conn, msg)) { + if (req_filter(conn, msg)) { return; } diff --git a/src/nc_sentinel.c b/src/nc_sentinel.c index e5d86686..bd8c81de 100644 --- a/src/nc_sentinel.c +++ b/src/nc_sentinel.c @@ -1,6 +1,7 @@ #include #include #include +#include static char *sentinel_reqs[] = { INFO_SENTINEL, diff --git a/src/nc_server.c b/src/nc_server.c index 73387f4e..eb9bdc9c 100644 --- a/src/nc_server.c +++ b/src/nc_server.c @@ -94,7 +94,7 @@ server_timeout(struct conn *conn) } bool -server_active(struct conn *conn) +server_active(const struct conn *conn) { ASSERT(!conn->client && !conn->proxy); @@ -827,7 +827,7 @@ server_pool_sentinel_check(struct context *ctx, struct server_pool *pool) } static uint32_t -server_pool_hash(struct server_pool *pool, uint8_t *key, uint32_t keylen) +server_pool_hash(const struct server_pool *pool, const uint8_t *key, uint32_t keylen) { ASSERT(array_n(&pool->server) != 0); ASSERT(key != NULL); @@ -840,11 +840,11 @@ server_pool_hash(struct server_pool *pool, uint8_t *key, uint32_t keylen) return 0; } - return pool->key_hash((char *)key, keylen); + return pool->key_hash((const char *)key, keylen); } uint32_t -server_pool_idx(struct server_pool *pool, uint8_t *key, uint32_t keylen) +server_pool_idx(const struct server_pool *pool, const uint8_t *key, uint32_t keylen) { uint32_t hash, idx; uint32_t nservers = array_n(&pool->server); @@ -863,8 +863,8 @@ server_pool_idx(struct server_pool *pool, uint8_t *key, uint32_t keylen) * we use the full key */ if (!string_empty(&pool->hash_tag)) { - struct string *tag = &pool->hash_tag; - uint8_t *tag_start, *tag_end; + const struct string *tag = &pool->hash_tag; + const uint8_t *tag_start, *tag_end; tag_start = nc_strchr(key, key + keylen, tag->data[0]); if (tag_start != NULL) { @@ -900,7 +900,7 @@ server_pool_idx(struct server_pool *pool, uint8_t *key, uint32_t keylen) } static struct server * -server_pool_server(struct server_pool *pool, uint8_t *key, uint32_t keylen) +server_pool_server(struct server_pool *pool, const uint8_t *key, uint32_t keylen) { struct server *server; uint32_t idx; @@ -918,7 +918,7 @@ server_pool_server(struct server_pool *pool, uint8_t *key, uint32_t keylen) * Returns a connection or null to forward the given key to. This will recursively choose a failover pool. */ static struct server * -server_pool_conn_failover(struct server_pool *failover, uint8_t *key, +server_pool_conn_failover(struct server_pool *failover, const uint8_t *key, uint32_t keylen) { /* Fallback to the failover pool */ @@ -955,7 +955,7 @@ server_pool_conn_failover(struct server_pool *failover, uint8_t *key, * This is called for every key in a memcache/redis request. */ struct conn * -server_pool_conn(struct context *ctx, struct server_pool *pool, uint8_t *key, +server_pool_conn(struct context *ctx, struct server_pool *pool, const uint8_t *key, uint32_t keylen) { rstatus_t status; diff --git a/src/nc_server.h b/src/nc_server.h index 1921d001..70286650 100644 --- a/src/nc_server.h +++ b/src/nc_server.h @@ -142,7 +142,7 @@ struct server_pool { void server_ref(struct conn *conn, void *owner); void server_unref(struct conn *conn); int server_timeout(struct conn *conn); -bool server_active(struct conn *conn); +bool server_active(const struct conn *conn); rstatus_t server_init(struct array *server, struct array *conf_server, struct server_pool *sp, bool sentinel); void server_deinit(struct array *server); struct conn *server_conn(struct server *server); @@ -153,8 +153,8 @@ void server_ok(struct context *ctx, struct conn *conn); struct server* server_find_by_name(struct context *ctx, struct server_pool *server_pool, struct string *server_name); rstatus_t server_switch(struct context *ctx, struct server *server, struct string *server_ip, int server_port); -uint32_t server_pool_idx(struct server_pool *pool, uint8_t *key, uint32_t keylen); -struct conn *server_pool_conn(struct context *ctx, struct server_pool *pool, uint8_t *key, uint32_t keylen); +uint32_t server_pool_idx(const struct server_pool *pool, const uint8_t *key, uint32_t keylen); +struct conn *server_pool_conn(struct context *ctx, struct server_pool *pool, const uint8_t *key, uint32_t keylen); rstatus_t server_pool_run(struct server_pool *pool); rstatus_t server_pool_connect(struct context *ctx); void server_pool_disconnect(struct context *ctx); diff --git a/src/nc_signal.c b/src/nc_signal.c index dd579cd1..a1cc54f3 100644 --- a/src/nc_signal.c +++ b/src/nc_signal.c @@ -21,7 +21,7 @@ #include #include -static struct signal signals[] = { +static const struct signal signals[] = { { SIGUSR1, "SIGUSR1", 0, signal_handler }, { SIGUSR2, "SIGUSR2", 0, signal_handler }, { SIGTTIN, "SIGTTIN", 0, signal_handler }, @@ -36,7 +36,7 @@ static struct signal signals[] = { rstatus_t signal_init(void) { - struct signal *sig; + const struct signal *sig; for (sig = signals; sig->signo != 0; sig++) { rstatus_t status; @@ -66,7 +66,7 @@ signal_deinit(void) void signal_handler(int signo) { - struct signal *sig; + const struct signal *sig; void (*action)(void); char *actionstr; bool done; diff --git a/src/nc_stats.c b/src/nc_stats.c index 00deb7f7..1c5a0f7a 100644 --- a/src/nc_stats.c +++ b/src/nc_stats.c @@ -42,11 +42,11 @@ static struct stats_metric stats_server_codec[] = { #undef DEFINE_ACTION #define DEFINE_ACTION(_name, _type, _desc) { .name = #_name, .desc = _desc }, -static struct stats_desc stats_pool_desc[] = { +static const struct stats_desc stats_pool_desc[] = { STATS_POOL_CODEC( DEFINE_ACTION ) }; -static struct stats_desc stats_server_desc[] = { +static const struct stats_desc stats_server_desc[] = { STATS_SERVER_CODEC( DEFINE_ACTION ) }; #undef DEFINE_ACTION @@ -188,7 +188,7 @@ stats_server_init(struct stats_server *sts, struct server *s) } static rstatus_t -stats_server_map(struct array *stats_server, struct array *server) +stats_server_map(struct array *stats_server, const struct array *server) { rstatus_t status; uint32_t i, nserver; @@ -233,7 +233,7 @@ stats_server_unmap(struct array *stats_server) } static rstatus_t -stats_pool_init(struct stats_pool *stp, struct server_pool *sp) +stats_pool_init(struct stats_pool *stp, const struct server_pool *sp) { rstatus_t status; @@ -281,7 +281,7 @@ stats_pool_reset(struct array *stats_pool) } static rstatus_t -stats_pool_map(struct array *stats_pool, struct array *server_pool) +stats_pool_map(struct array *stats_pool, const struct array *server_pool) { rstatus_t status; uint32_t i, npool; @@ -295,7 +295,7 @@ stats_pool_map(struct array *stats_pool, struct array *server_pool) } for (i = 0; i < npool; i++) { - struct server_pool *sp = array_get(server_pool, i); + const struct server_pool *sp = array_get(server_pool, i); struct stats_pool *stp = array_push(stats_pool); status = stats_pool_init(stp, sp); @@ -442,7 +442,7 @@ stats_destroy_buf(struct stats *st) } static rstatus_t -stats_add_string(struct stats *st, struct string *key, struct string *val) +stats_add_string(struct stats *st, const struct string *key, const struct string *val) { struct stats_buffer *buf; uint8_t *pos; @@ -487,7 +487,7 @@ stats_add_hardcoded_string(struct stats *st, const char* key, const char* value) } static rstatus_t -stats_add_num(struct stats *st, struct string *key, int64_t val) +stats_add_num(struct stats *st, const struct string *key, int64_t val) { struct stats_buffer *buf; uint8_t *pos; @@ -588,7 +588,7 @@ stats_add_footer(struct stats *st) } static rstatus_t -stats_begin_nesting(struct stats *st, struct string *key) +stats_begin_nesting(struct stats *st, const struct string *key) { struct stats_buffer *buf; uint8_t *pos; @@ -665,12 +665,13 @@ stats_copy_metric(struct stats *st, struct array *metric) } static void -stats_aggregate_metric(struct array *dst, struct array *src) +stats_aggregate_metric(struct array *dst, const struct array *src) { uint32_t i; for (i = 0; i < array_n(src); i++) { - struct stats_metric *stm1, *stm2; + const struct stats_metric *stm1; + struct stats_metric *stm2; stm1 = array_get(src, i); stm2 = array_get(dst, i); @@ -948,8 +949,8 @@ stats_stop_aggregator(struct stats *st) } struct stats * -stats_create(uint16_t stats_port, char *stats_ip, int stats_interval, - char *source, struct array *server_pool) +stats_create(uint16_t stats_port, const char *stats_ip, int stats_interval, + const char *source, const struct array *server_pool) { rstatus_t status; struct stats *st; @@ -1076,7 +1077,7 @@ stats_swap(struct stats *st) } static struct stats_metric * -stats_pool_to_metric(struct context *ctx, struct server_pool *pool, +stats_pool_to_metric(struct context *ctx, const struct server_pool *pool, stats_pool_field_t fidx) { struct stats *st; @@ -1099,7 +1100,7 @@ stats_pool_to_metric(struct context *ctx, struct server_pool *pool, } void -_stats_pool_incr(struct context *ctx, struct server_pool *pool, +_stats_pool_incr(struct context *ctx, const struct server_pool *pool, stats_pool_field_t fidx) { struct stats_metric *stm; @@ -1114,7 +1115,7 @@ _stats_pool_incr(struct context *ctx, struct server_pool *pool, } void -_stats_pool_decr(struct context *ctx, struct server_pool *pool, +_stats_pool_decr(struct context *ctx, const struct server_pool *pool, stats_pool_field_t fidx) { struct stats_metric *stm; @@ -1129,7 +1130,7 @@ _stats_pool_decr(struct context *ctx, struct server_pool *pool, } void -_stats_pool_incr_by(struct context *ctx, struct server_pool *pool, +_stats_pool_incr_by(struct context *ctx, const struct server_pool *pool, stats_pool_field_t fidx, int64_t val) { struct stats_metric *stm; @@ -1144,7 +1145,7 @@ _stats_pool_incr_by(struct context *ctx, struct server_pool *pool, } void -_stats_pool_decr_by(struct context *ctx, struct server_pool *pool, +_stats_pool_decr_by(struct context *ctx, const struct server_pool *pool, stats_pool_field_t fidx, int64_t val) { struct stats_metric *stm; @@ -1159,7 +1160,7 @@ _stats_pool_decr_by(struct context *ctx, struct server_pool *pool, } void -_stats_pool_set_ts(struct context *ctx, struct server_pool *pool, +_stats_pool_set_ts(struct context *ctx, const struct server_pool *pool, stats_pool_field_t fidx, int64_t val) { struct stats_metric *stm; @@ -1174,7 +1175,7 @@ _stats_pool_set_ts(struct context *ctx, struct server_pool *pool, } static struct stats_metric * -stats_server_to_metric(struct context *ctx, struct server *server, +stats_server_to_metric(struct context *ctx, const struct server *server, stats_server_field_t fidx) { struct stats *st; @@ -1200,7 +1201,7 @@ stats_server_to_metric(struct context *ctx, struct server *server, } void -_stats_server_incr(struct context *ctx, struct server *server, +_stats_server_incr(struct context *ctx, const struct server *server, stats_server_field_t fidx) { struct stats_metric *stm; @@ -1220,7 +1221,7 @@ _stats_server_incr(struct context *ctx, struct server *server, } void -_stats_server_decr(struct context *ctx, struct server *server, +_stats_server_decr(struct context *ctx, const struct server *server, stats_server_field_t fidx) { struct stats_metric *stm; @@ -1240,7 +1241,7 @@ _stats_server_decr(struct context *ctx, struct server *server, } void -_stats_server_incr_by(struct context *ctx, struct server *server, +_stats_server_incr_by(struct context *ctx, const struct server *server, stats_server_field_t fidx, int64_t val) { struct stats_metric *stm; @@ -1260,7 +1261,7 @@ _stats_server_incr_by(struct context *ctx, struct server *server, } void -_stats_server_decr_by(struct context *ctx, struct server *server, +_stats_server_decr_by(struct context *ctx, const struct server *server, stats_server_field_t fidx, int64_t val) { struct stats_metric *stm; @@ -1280,7 +1281,7 @@ _stats_server_decr_by(struct context *ctx, struct server *server, } void -_stats_server_set_ts(struct context *ctx, struct server *server, +_stats_server_set_ts(struct context *ctx, const struct server *server, stats_server_field_t fidx, int64_t val) { struct stats_metric *stm; diff --git a/src/nc_stats.h b/src/nc_stats.h index 4db128c8..f0c4f6ac 100644 --- a/src/nc_stats.h +++ b/src/nc_stats.h @@ -198,19 +198,19 @@ typedef enum stats_server_field { void stats_describe(void); -void _stats_pool_incr(struct context *ctx, struct server_pool *pool, stats_pool_field_t fidx); -void _stats_pool_decr(struct context *ctx, struct server_pool *pool, stats_pool_field_t fidx); -void _stats_pool_incr_by(struct context *ctx, struct server_pool *pool, stats_pool_field_t fidx, int64_t val); -void _stats_pool_decr_by(struct context *ctx, struct server_pool *pool, stats_pool_field_t fidx, int64_t val); -void _stats_pool_set_ts(struct context *ctx, struct server_pool *pool, stats_pool_field_t fidx, int64_t val); - -void _stats_server_incr(struct context *ctx, struct server *server, stats_server_field_t fidx); -void _stats_server_decr(struct context *ctx, struct server *server, stats_server_field_t fidx); -void _stats_server_incr_by(struct context *ctx, struct server *server, stats_server_field_t fidx, int64_t val); -void _stats_server_decr_by(struct context *ctx, struct server *server, stats_server_field_t fidx, int64_t val); -void _stats_server_set_ts(struct context *ctx, struct server *server, stats_server_field_t fidx, int64_t val); - -struct stats *stats_create(uint16_t stats_port, char *stats_ip, int stats_interval, char *source, struct array *server_pool); +void _stats_pool_incr(struct context *ctx, const struct server_pool *pool, stats_pool_field_t fidx); +void _stats_pool_decr(struct context *ctx, const struct server_pool *pool, stats_pool_field_t fidx); +void _stats_pool_incr_by(struct context *ctx, const struct server_pool *pool, stats_pool_field_t fidx, int64_t val); +void _stats_pool_decr_by(struct context *ctx, const struct server_pool *pool, stats_pool_field_t fidx, int64_t val); +void _stats_pool_set_ts(struct context *ctx, const struct server_pool *pool, stats_pool_field_t fidx, int64_t val); + +void _stats_server_incr(struct context *ctx, const struct server *server, stats_server_field_t fidx); +void _stats_server_decr(struct context *ctx, const struct server *server, stats_server_field_t fidx); +void _stats_server_incr_by(struct context *ctx, const struct server *server, stats_server_field_t fidx, int64_t val); +void _stats_server_decr_by(struct context *ctx, const struct server *server, stats_server_field_t fidx, int64_t val); +void _stats_server_set_ts(struct context *ctx, const struct server *server, stats_server_field_t fidx, int64_t val); + +struct stats *stats_create(uint16_t stats_port, const char *stats_ip, int stats_interval, const char *source, const struct array *server_pool); void stats_destroy(struct stats *stats); void stats_swap(struct stats *stats); diff --git a/src/nc_string.c b/src/nc_string.c index 79a4a203..90dfea02 100644 --- a/src/nc_string.c +++ b/src/nc_string.c @@ -61,7 +61,7 @@ string_empty(const struct string *str) { ASSERT((str->len == 0 && str->data == NULL) || (str->len != 0 && str->data != NULL)); - return str->len == 0 ? true : false; + return str->len == 0; } rstatus_t @@ -119,10 +119,11 @@ string_has_prefix(const struct string *s1, const struct string *prefix) return nc_strncmp(s1->data, prefix->data, prefix->len) == 0; } +static const char *const hex = "0123456789abcdef"; + static char * _safe_utoa(int _base, uint64_t val, char *buf) { - char hex[] = "0123456789abcdef"; uint32_t base = (uint32_t) _base; *buf-- = 0; do { @@ -134,7 +135,6 @@ _safe_utoa(int _base, uint64_t val, char *buf) static char * _safe_itoa(int base, int64_t val, char *buf) { - char hex[] = "0123456789abcdef"; char *orig_buf = buf; const int32_t is_neg = (val < 0); *buf-- = 0; diff --git a/src/nc_util.c b/src/nc_util.c index 884db231..7f642d56 100644 --- a/src/nc_util.c +++ b/src/nc_util.c @@ -239,7 +239,7 @@ nc_get_rcvbuf(int sd) } int -_nc_atoi(uint8_t *line, size_t n) +_nc_atoi(const uint8_t *line, size_t n) { int value; @@ -513,7 +513,7 @@ nc_msec_now(void) } static int -nc_resolve_inet(struct string *name, int port, struct sockinfo *si) +nc_resolve_inet(const struct string *name, int port, struct sockinfo *si) { int status; struct addrinfo *ai, *cai; /* head and current addrinfo */ @@ -583,7 +583,7 @@ nc_resolve_inet(struct string *name, int port, struct sockinfo *si) } static int -nc_resolve_unix(struct string *name, struct sockinfo *si) +nc_resolve_unix(const struct string *name, struct sockinfo *si) { struct sockaddr_un *un; @@ -611,7 +611,7 @@ nc_resolve_unix(struct string *name, struct sockinfo *si) * This routine is reentrant */ int -nc_resolve(struct string *name, int port, struct sockinfo *si) +nc_resolve(const struct string *name, int port, struct sockinfo *si) { if (name != NULL && name->data[0] == '/') { return nc_resolve_unix(name, si); @@ -626,7 +626,7 @@ nc_resolve(struct string *name, int port, struct sockinfo *si) * * This routine is not reentrant */ -char * +const char * nc_unresolve_addr(struct sockaddr *addr, socklen_t addrlen) { static char unresolve[NI_MAXHOST + NI_MAXSERV]; @@ -651,7 +651,7 @@ nc_unresolve_addr(struct sockaddr *addr, socklen_t addrlen) * * This routine is not reentrant */ -char * +const char * nc_unresolve_peer_desc(int sd) { static struct sockinfo si; @@ -677,7 +677,7 @@ nc_unresolve_peer_desc(int sd) * * This routine is not reentrant */ -char * +const char * nc_unresolve_desc(int sd) { static struct sockinfo si; diff --git a/src/nc_util.h b/src/nc_util.h index f805449e..97e24440 100644 --- a/src/nc_util.h +++ b/src/nc_util.h @@ -20,6 +20,18 @@ #include +#ifdef __GNUC__ +# define NC_GCC_VERSION (__GNUC__ * 1000 + __GNUC_MINOR__) +#else +# define NC_GCC_VERSION 0 +#endif +#if NC_GCC_VERSION >= 2007 +#define NC_ATTRIBUTE_FORMAT(type, idx, first) __attribute__ ((format(type, idx, first))) +#else +#define NC_ATTRIBUTE_FORMAT(type, idx, first) +#endif + + #define LF (uint8_t) 10 #define CR (uint8_t) 13 #define CRLF "\x0d\x0a" @@ -90,7 +102,7 @@ int nc_get_soerror(int sd); int nc_get_sndbuf(int sd); int nc_get_rcvbuf(int sd); -int _nc_atoi(uint8_t *line, size_t n); +int _nc_atoi(const uint8_t *line, size_t n); bool nc_valid_port(int n); /* @@ -188,7 +200,7 @@ void nc_assert(const char *cond, const char *file, int line, int panic); void nc_stacktrace(int skip_count); void nc_stacktrace_fd(int fd); -int _scnprintf(char *buf, size_t size, const char *fmt, ...); +int _scnprintf(char *buf, size_t size, const char *fmt, ...) NC_ATTRIBUTE_FORMAT(printf, 3, 4); int _vscnprintf(char *buf, size_t size, const char *fmt, va_list args); int64_t nc_usec_now(void); int64_t nc_msec_now(void); @@ -208,9 +220,9 @@ struct sockinfo { } addr; }; -int nc_resolve(struct string *name, int port, struct sockinfo *si); -char *nc_unresolve_addr(struct sockaddr *addr, socklen_t addrlen); -char *nc_unresolve_peer_desc(int sd); -char *nc_unresolve_desc(int sd); +int nc_resolve(const struct string *name, int port, struct sockinfo *si); +const char *nc_unresolve_addr(struct sockaddr *addr, socklen_t addrlen); +const char *nc_unresolve_peer_desc(int sd); +const char *nc_unresolve_desc(int sd); #endif diff --git a/src/proto/nc_memcache.c b/src/proto/nc_memcache.c index 17e1a5d8..87e5584a 100644 --- a/src/proto/nc_memcache.c +++ b/src/proto/nc_memcache.c @@ -37,7 +37,7 @@ * return false */ static bool -memcache_storage(struct msg *r) +memcache_storage(const struct msg *r) { switch (r->type) { case MSG_REQ_MC_SET: @@ -60,7 +60,7 @@ memcache_storage(struct msg *r) * return false */ static bool -memcache_cas(struct msg *r) +memcache_cas(const struct msg *r) { if (r->type == MSG_REQ_MC_CAS) { return true; @@ -74,7 +74,7 @@ memcache_cas(struct msg *r) * return false */ static bool -memcache_retrieval(struct msg *r) +memcache_retrieval(const struct msg *r) { switch (r->type) { case MSG_REQ_MC_GET: @@ -93,19 +93,23 @@ memcache_retrieval(struct msg *r) * otherwise return false. * * The only supported memcache commands that can have multiple keys - * are get/gets. Both are multigets, and the latter returns CAS token with the value. + * are get/gets. Both are multigets, and the latter returns CAS token with the + * value. * - * Fragmented requests are assumed to be slower due to the fact that they need to allocate - * an array to track which key went to which server, so avoid them when possible. + * Fragmented requests are assumed to be slower due to the fact that they need + * to allocate an array to track which key went to which server, + * so avoid them when possible. */ static bool -memcache_should_fragment(struct msg *r) +memcache_should_fragment(const struct msg *r) { switch (r->type) { case MSG_REQ_MC_GET: case MSG_REQ_MC_GETS: - // A memcache get for a single key is only sent to one server. - // Fragmenting it would work but be less efficient. + /* + * A memcache get for a single key is only sent to one server. + * Fragmenting it would work but be less efficient. + */ return array_n(r->keys) != 1; default: @@ -120,7 +124,7 @@ memcache_should_fragment(struct msg *r) * return false */ static bool -memcache_arithmetic(struct msg *r) +memcache_arithmetic(const struct msg *r) { switch (r->type) { case MSG_REQ_MC_INCR: @@ -139,7 +143,7 @@ memcache_arithmetic(struct msg *r) * return false */ static bool -memcache_delete(struct msg *r) +memcache_delete(const struct msg *r) { if (r->type == MSG_REQ_MC_DELETE) { return true; @@ -153,7 +157,7 @@ memcache_delete(struct msg *r) * return false */ static bool -memcache_touch(struct msg *r) +memcache_touch(const struct msg *r) { if (r->type == MSG_REQ_MC_TOUCH) { return true; @@ -1228,13 +1232,13 @@ memcache_parse_rsp(struct msg *r) } bool -memcache_failure(struct msg *r) +memcache_failure(const struct msg *r) { return false; } static rstatus_t -memcache_append_key(struct msg *r, uint8_t *key, uint32_t keylen) +memcache_append_key(struct msg *r, const uint8_t *key, uint32_t keylen) { struct mbuf *mbuf; struct keypos *kpos; @@ -1254,7 +1258,7 @@ memcache_append_key(struct msg *r, uint8_t *key, uint32_t keylen) mbuf_copy(mbuf, key, keylen); r->mlen += keylen; - mbuf_copy(mbuf, (uint8_t *)" ", 1); + mbuf_copy(mbuf, (const uint8_t *)" ", 1); r->mlen += 1; return NC_OK; } @@ -1263,7 +1267,7 @@ memcache_append_key(struct msg *r, uint8_t *key, uint32_t keylen) * read the comment in proto/nc_redis.c */ static rstatus_t -memcache_fragment_retrieval(struct msg *r, uint32_t nservers, +memcache_fragment_retrieval(struct msg *r, uint32_t nserver, struct msg_tqh *frag_msgq, uint32_t key_step) { @@ -1345,17 +1349,17 @@ memcache_fragment_retrieval(struct msg *r, uint32_t nservers, return status; } - sub_msgs = nc_zalloc(nservers * sizeof(*sub_msgs)); + sub_msgs = nc_zalloc(nserver * sizeof(*sub_msgs)); if (sub_msgs == NULL) { return NC_ENOMEM; } - /** Build up the key1 key2 ... to be sent to a given server at index idx */ + /* Build up the key1 key2 ... to be sent to a given server at index idx */ for (i = 0; i < array_n(r->keys); i++) { /* for each key */ struct msg *sub_msg; struct keypos *kpos = array_get_known_type(r->keys, i, struct keypos); uint32_t idx = msg_backend_idx(r, kpos->start, kpos->end - kpos->start); - ASSERT(idx < nservers); + ASSERT(idx < nserver); if (sub_msgs[idx] == NULL) { sub_msgs[idx] = msg_get(r->owner, r->request, r->redis); @@ -1373,8 +1377,11 @@ memcache_fragment_retrieval(struct msg *r, uint32_t nservers, return status; } } - - for (i = 0; i < nservers; i++) { /* prepend mget header, and forward the get[s] key1 key2\r\n to the corresponding server(s) */ + /* + * prepend mget header, and forward the get[s] key1 key2\r\n + * to the corresponding server(s) + */ + for (i = 0; i < nserver; i++) { struct msg *sub_msg = sub_msgs[i]; if (sub_msg == NULL) { continue; @@ -1411,10 +1418,10 @@ memcache_fragment_retrieval(struct msg *r, uint32_t nservers, } rstatus_t -memcache_fragment(struct msg *r, uint32_t nservers, struct msg_tqh *frag_msgq) +memcache_fragment(struct msg *r, uint32_t nserver, struct msg_tqh *frag_msgq) { if (memcache_should_fragment(r)) { - return memcache_fragment_retrieval(r, nservers, frag_msgq, 1); + return memcache_fragment_retrieval(r, nserver, frag_msgq, 1); } return NC_OK; } diff --git a/src/proto/nc_proto.h b/src/proto/nc_proto.h index 78ee2a89..6470f7b7 100644 --- a/src/proto/nc_proto.h +++ b/src/proto/nc_proto.h @@ -144,22 +144,22 @@ void memcache_parse_req(struct msg *r); void memcache_parse_rsp(struct msg *r); -bool memcache_failure(struct msg *r); +bool memcache_failure(const struct msg *r); void memcache_pre_coalesce(struct msg *r); void memcache_post_coalesce(struct msg *r); rstatus_t memcache_add_auth(struct context *ctx, struct conn *c_conn, struct conn *s_conn); -rstatus_t memcache_fragment(struct msg *r, uint32_t nservers, struct msg_tqh *frag_msgq); +rstatus_t memcache_fragment(struct msg *r, uint32_t nserver, struct msg_tqh *frag_msgq); rstatus_t memcache_reply(struct msg *r); void memcache_post_connect(struct context *ctx, struct conn *conn, struct server *server); void memcache_swallow_msg(struct conn *conn, struct msg *pmsg, struct msg *msg); void redis_parse_req(struct msg *r); void redis_parse_rsp(struct msg *r); -bool redis_failure(struct msg *r); +bool redis_failure(const struct msg *r); void redis_pre_coalesce(struct msg *r); void redis_post_coalesce(struct msg *r); rstatus_t redis_add_auth(struct context *ctx, struct conn *c_conn, struct conn *s_conn); -rstatus_t redis_fragment(struct msg *r, uint32_t nservers, struct msg_tqh *frag_msgq); +rstatus_t redis_fragment(struct msg *r, uint32_t nserver, struct msg_tqh *frag_msgq); rstatus_t redis_reply(struct msg *r); void redis_post_connect(struct context *ctx, struct conn *conn, struct server *server); void redis_swallow_msg(struct conn *conn, struct msg *pmsg, struct msg *msg); diff --git a/src/proto/nc_redis.c b/src/proto/nc_redis.c index 244396bf..64405c78 100644 --- a/src/proto/nc_redis.c +++ b/src/proto/nc_redis.c @@ -40,7 +40,7 @@ static rstatus_t redis_handle_auth_req(struct msg *request, struct msg *response * return false */ static bool -redis_argz(struct msg *r) +redis_argz(const struct msg *r) { switch (r->type) { /* TODO: PING has an optional argument, emulate that? */ @@ -61,7 +61,7 @@ redis_argz(struct msg *r) * return false */ static bool -redis_arg0(struct msg *r) +redis_arg0(const struct msg *r) { switch (r->type) { case MSG_REQ_REDIS_PERSIST: @@ -103,7 +103,7 @@ redis_arg0(struct msg *r) * return false */ static bool -redis_arg1(struct msg *r) +redis_arg1(const struct msg *r) { switch (r->type) { case MSG_REQ_REDIS_EXPIRE: @@ -146,7 +146,7 @@ redis_arg1(struct msg *r) * return false */ static bool -redis_arg2(struct msg *r) +redis_arg2(const struct msg *r) { switch (r->type) { case MSG_REQ_REDIS_GETRANGE: @@ -189,7 +189,7 @@ redis_arg2(struct msg *r) * return false */ static bool -redis_arg3(struct msg *r) +redis_arg3(const struct msg *r) { switch (r->type) { case MSG_REQ_REDIS_LINSERT: @@ -208,7 +208,7 @@ redis_arg3(struct msg *r) * return false */ static bool -redis_argn(struct msg *r) +redis_argn(const struct msg *r) { switch (r->type) { case MSG_REQ_REDIS_SORT: @@ -259,11 +259,6 @@ redis_argn(struct msg *r) case MSG_REQ_REDIS_PFMERGE: case MSG_REQ_REDIS_PFCOUNT: -#if SUPPORT_BLOCKING_REDIS_COMMAND_UNSAFE - case MSG_REQ_REDIS_BZPOPMAX: - case MSG_REQ_REDIS_BZPOPMIN: -#endif - case MSG_REQ_REDIS_ZADD: case MSG_REQ_REDIS_ZDIFF: case MSG_REQ_REDIS_ZDIFFSTORE: @@ -309,7 +304,7 @@ redis_argn(struct msg *r) * more keys, otherwise return false */ static bool -redis_argx(struct msg *r) +redis_argx(const struct msg *r) { switch (r->type) { case MSG_REQ_REDIS_MGET: @@ -330,7 +325,7 @@ redis_argx(struct msg *r) * more key-value pairs, otherwise return false */ static bool -redis_argkvx(struct msg *r) +redis_argkvx(const struct msg *r) { switch (r->type) { case MSG_REQ_REDIS_MSET: @@ -350,7 +345,7 @@ redis_argkvx(struct msg *r) * that at least one argument is required, but that shouldn't be the case). */ static bool -redis_argeval(struct msg *r) +redis_argeval(const struct msg *r) { switch (r->type) { case MSG_REQ_REDIS_EVAL: @@ -365,7 +360,7 @@ redis_argeval(struct msg *r) } static bool -redis_nokey(struct msg *r) +redis_nokey(const struct msg *r) { switch (r->type) { case MSG_REQ_REDIS_LOLWUT: @@ -383,7 +378,7 @@ redis_nokey(struct msg *r) * string whose first character is '-', otherwise return false. */ static bool -redis_error(struct msg *r) +redis_error(const struct msg *r) { switch (r->type) { case MSG_RSP_REDIS_ERROR: @@ -409,7 +404,10 @@ redis_error(struct msg *r) return false; } -// Set a placeholder key for a command with no key that is forwarded to an arbitrary backend. +/* + * Set a placeholder key for a command with no key that is forwarded to an + * arbitrary backend. + */ static bool set_placeholder_key(struct msg *r) { @@ -890,18 +888,6 @@ redis_parse_req(struct msg *r) break; } -#if SUPPORT_BLOCKING_REDIS_COMMAND_UNSAFE - if (str5icmp(m, 'b', 'l', 'p', 'o', 'p')) { - r->type = MSG_REQ_REDIS_BLPOP; - break; - } - - if (str5icmp(m, 'b', 'r', 'p', 'o', 'p')) { - r->type = MSG_REQ_REDIS_BRPOP; - break; - } -#endif - break; case 6: @@ -1182,18 +1168,6 @@ redis_parse_req(struct msg *r) break; } -#if SUPPORT_BLOCKING_REDIS_COMMAND_UNSAFE - if (str8icmp(m, 'b', 'z', 'p', 'o', 'p', 'm', 'i', 'n')) { - r->type = MSG_REQ_REDIS_BZPOPMIN; - break; - } - - if (str8icmp(m, 'b', 'z', 'p', 'o', 'p', 'm', 'a', 'x')) { - r->type = MSG_REQ_REDIS_BZPOPMAX; - break; - } -#endif - break; case 9: @@ -1255,13 +1229,6 @@ redis_parse_req(struct msg *r) break; } -#if SUPPORT_BLOCKING_REDIS_COMMAND_UNSAFE - if (str10icmp(m, 'b', 'r', 'p', 'o', 'p', 'l', 'p', 'u', 's', 'h')) { - r->type = MSG_REQ_REDIS_BRPOPLPUSH; - break; - } -#endif - break; case 11: @@ -2218,7 +2185,7 @@ redis_parse_rsp(struct msg *r) if (ch == '\r') { state = SW_ALMOST_DONE; } else { - // Read remaining characters until '\r' + /* Read remaining characters until '\r' */ state = SW_RUNTO_CRLF; } } @@ -2276,8 +2243,11 @@ redis_parse_rsp(struct msg *r) break; case SW_BULK: - /* SW_BULK is used for top-level bulk string replies. */ - /* Within an array, SW_MULTIBULK_ARG... helpers are used to parse bulk strings instead. */ + /* + * SW_BULK is used for top-level bulk string replies. + * Within an array, SW_MULTIBULK_ARG... helpers are used + * to parse bulk strings instead. + */ if (r->token == NULL) { if (ch != '$') { goto error; @@ -2359,8 +2329,10 @@ redis_parse_rsp(struct msg *r) } else if (ch == '-') { p = p-1; r->token = NULL; - // This is a null array (e.g. from BLPOP). Don't increment rnarg - // https://redis.io/topics/protocol + /* + * This is a null array (e.g. from BLPOP). Don't increment rnarg + * https://redis.io/topics/protocol + */ r->vlen = 1; state = SW_MULTIBULK_ARGN_LEN; } else if (isdigit(ch)) { @@ -2380,7 +2352,10 @@ redis_parse_rsp(struct msg *r) r->rnarg += r->vlen - 1; r->token = NULL; - // The stack is always initialized before transitioning to another state. + /* + * The stack is always initialized before transitioning + * to another state. + */ state = SW_MULTIBULK_NARG_LF; } else { goto error; @@ -2583,7 +2558,7 @@ redis_parse_rsp(struct msg *r) * See issue: https://github.com/twitter/twemproxy/issues/369 */ bool -redis_failure(struct msg *r) +redis_failure(const struct msg *r) { ASSERT(!r->request); @@ -2763,7 +2738,7 @@ redis_pre_coalesce(struct msg *r) } static rstatus_t -redis_append_key(struct msg *r, uint8_t *key, uint32_t keylen) +redis_append_key(struct msg *r, const uint8_t *key, uint32_t keylen) { uint32_t len; struct mbuf *mbuf; @@ -2808,9 +2783,9 @@ redis_append_key(struct msg *r, uint8_t *key, uint32_t keylen) /* * input a msg, return a msg chain. - * nservers is the number of backend redis/memcache server + * nserver is the number of backend redis/memcache server * - * the original msg will be fragmented into at most nservers fragments. + * the original msg will be fragmented into at most nserver fragments. * all the keys map to the same backend will group into one fragment. * * frag_id: @@ -2859,7 +2834,7 @@ redis_append_key(struct msg *r, uint8_t *key, uint32_t keylen) * */ static rstatus_t -redis_fragment_argx(struct msg *r, uint32_t nservers, struct msg_tqh *frag_msgq, +redis_fragment_argx(struct msg *r, uint32_t nserver, struct msg_tqh *frag_msgq, uint32_t key_step) { struct mbuf *mbuf; @@ -2870,7 +2845,7 @@ redis_fragment_argx(struct msg *r, uint32_t nservers, struct msg_tqh *frag_msgq, ASSERT(array_n(keys) == (r->narg - 1) / key_step); - sub_msgs = nc_zalloc(nservers * sizeof(*sub_msgs)); + sub_msgs = nc_zalloc(nserver * sizeof(*sub_msgs)); if (sub_msgs == NULL) { return NC_ENOMEM; } @@ -2902,12 +2877,12 @@ redis_fragment_argx(struct msg *r, uint32_t nservers, struct msg_tqh *frag_msgq, r->nfrag = 0; r->frag_owner = r; - /** Build up the key1 key2 ... to be sent to a given server at index idx */ + /* Build up the key1 key2 ... to be sent to a given server at index idx */ for (i = 0; i < array_n(keys); i++) { /* for each key */ struct msg *sub_msg; - struct keypos *kpos = array_get_known_type(keys, i, struct keypos); + struct keypos *kpos = array_get(keys, i); uint32_t idx = msg_backend_idx(r, kpos->start, kpos->end - kpos->start); - ASSERT(idx < nservers); + ASSERT(idx < nserver); if (sub_msgs[idx] == NULL) { sub_msgs[idx] = msg_get(r->owner, r->request, r->redis); @@ -2944,7 +2919,11 @@ redis_fragment_argx(struct msg *r, uint32_t nservers, struct msg_tqh *frag_msgq, } } - for (i = 0; i < nservers; i++) { /* prepend mget header, and forward the command (command type+key(s)+suffix) to the corresponding server(s) */ + /* + * prepend mget header, and forward the command (command type+key(s)+suffix) + * to the corresponding server(s) + */ + for (i = 0; i < nserver; i++) { struct msg *sub_msg = sub_msgs[i]; if (sub_msg == NULL) { continue; @@ -2986,7 +2965,7 @@ redis_fragment_argx(struct msg *r, uint32_t nservers, struct msg_tqh *frag_msgq, } rstatus_t -redis_fragment(struct msg *r, uint32_t nservers, struct msg_tqh *frag_msgq) +redis_fragment(struct msg *r, uint32_t nserver, struct msg_tqh *frag_msgq) { if (1 == array_n(r->keys)){ return NC_OK; @@ -2997,11 +2976,11 @@ redis_fragment(struct msg *r, uint32_t nservers, struct msg_tqh *frag_msgq) case MSG_REQ_REDIS_DEL: case MSG_REQ_REDIS_TOUCH: case MSG_REQ_REDIS_UNLINK: - return redis_fragment_argx(r, nservers, frag_msgq, 1); + return redis_fragment_argx(r, nserver, frag_msgq, 1); /* TODO: MSETNX - instead of responding with OK, respond with 1 if all fragments respond with 1 */ case MSG_REQ_REDIS_MSET: - return redis_fragment_argx(r, nservers, frag_msgq, 2); + return redis_fragment_argx(r, nserver, frag_msgq, 2); default: return NC_OK; @@ -3132,15 +3111,15 @@ static rstatus_t redis_handle_auth_req(struct msg *req, struct msg *rsp) { struct conn *conn = (struct conn *)rsp->owner; - struct server_pool *pool; - struct keypos *kpos; - uint8_t *key; + const struct server_pool *pool; + const struct keypos *kpos; + const uint8_t *key; uint32_t keylen; bool valid; ASSERT(conn->client && !conn->proxy); - pool = (struct server_pool *)conn->owner; + pool = (const struct server_pool *)conn->owner; if (!pool->require_auth) { /* @@ -3154,7 +3133,7 @@ redis_handle_auth_req(struct msg *req, struct msg *rsp) key = kpos->start; keylen = (uint32_t)(kpos->end - kpos->start); valid = (keylen == pool->redis_auth.len) && - (memcmp(pool->redis_auth.data, key, keylen) == 0) ? true : false; + (memcmp(pool->redis_auth.data, key, keylen) == 0); if (valid) { conn->authenticated = 1; return msg_append(rsp, rsp_ok.data, rsp_ok.len); diff --git a/src/test_all.c b/src/test_all.c index e5ce6c2e..5c15dc77 100644 --- a/src/test_all.c +++ b/src/test_all.c @@ -2,16 +2,28 @@ #include #include #include +#include static int failures = 0; static int successes = 0; +static void expect_same_int(int expected, int actual, const char* message) { + if (expected != actual) { + printf("FAIL Expected %d, got %d (%s)\n", expected, actual, message); + failures++; + } else { + /* printf("PASS (%s)\n", message); */ + successes++; + } +} + static void expect_same_uint32_t(uint32_t expected, uint32_t actual, const char* message) { if (expected != actual) { - printf("FAIL Expected %u, got %u (%s)\n", (unsigned int) expected, (unsigned int) actual, message); + printf("FAIL Expected %u, got %u (%s)\n", (unsigned int) expected, + (unsigned int) actual, message); failures++; } else { - // printf("PASS (%s)\n", message); + /* printf("PASS (%s)\n", message); */ successes++; } } @@ -21,13 +33,13 @@ static void expect_same_ptr(void *expected, void *actual, const char* message) { printf("FAIL Expected %p, got %p (%s)\n", expected, actual, message); failures++; } else { - // printf("PASS (%s)\n", message); + /* printf("PASS (%s)\n", message); */ successes++; } } static void test_hash_algorithms(void) { - // refer to libmemcached tests/hash_results.h + /* refer to libmemcached tests/hash_results.h */ expect_same_uint32_t(2297466611U, hash_one_at_a_time("apple", 5), "should have expected one_at_a_time hash for key \"apple\""); expect_same_uint32_t(3195025439U, hash_md5("apple", 5), "should have expected md5 hash for key \"apple\""); expect_same_uint32_t(3853726576U, ketama_hash("server1-8", strlen("server1-8"), 0), "should have expected ketama_hash for server1-8 index 0"); @@ -35,7 +47,7 @@ static void test_hash_algorithms(void) { } static void test_config_parsing(void) { - char* conf_file = "../conf/nutcracker.yml"; + const char* conf_file = "../conf/nutcracker.yml"; struct conf * conf = conf_create(conf_file); if (conf == NULL) { printf("FAIL could not parse %s (this test should be run within src/ folder)\n", conf_file); @@ -48,19 +60,19 @@ static void test_config_parsing(void) { } } -static void test_redis_parse_req_success_case(char* data, int expected_type) { +static void test_redis_parse_req_success_case(const char* data, int expected_type) { struct conn fake_client = {0}; struct mbuf *m = mbuf_get(); - const int SW_START = 0; // Same as SW_START in redis_parse_req + const int SW_START = 0; /* Same as SW_START in redis_parse_req */ struct msg *req = msg_get(&fake_client, 1, 1); req->state = SW_START; req->token = NULL; - const size_t datalen = (int)strlen(data); + const size_t datalen = strlen(data); - // Copy data into the message - mbuf_copy(m, (uint8_t*)data, datalen); - // Insert a single buffer into the message mbuf header + /* Copy data into the message */ + mbuf_copy(m, (const uint8_t*)data, datalen); + /* Insert a single buffer into the message mbuf header */ STAILQ_INIT(&req->mhdr); ASSERT(STAILQ_EMPTY(&req->mhdr)); mbuf_insert(&req->mhdr, m); @@ -68,43 +80,44 @@ static void test_redis_parse_req_success_case(char* data, int expected_type) { redis_parse_req(req); expect_same_ptr(m->last, req->pos, "redis_parse_req: expected req->pos to be m->last"); - expect_same_uint32_t(SW_START, req->state, "redis_parse_req: expected full buffer to be parsed"); - expect_same_uint32_t(expected_type, req->type, "redis_parse_req: expected request type to be parsed"); - expect_same_uint32_t(0, fake_client.err, "redis_parse_req: expected no connection error"); + expect_same_int(SW_START, req->state, "redis_parse_req: expected full buffer to be parsed"); + expect_same_int(expected_type, req->type, "redis_parse_req: expected request type to be parsed"); + expect_same_int(0, fake_client.err, "redis_parse_req: expected no connection error"); msg_put(req); - // mbuf_put(m); + /* mbuf_put(m); */ } -// Test support for https://redis.io/topics/protocol +/* Test support for https://redis.io/topics/protocol */ static void test_redis_parse_req_success(void) { - // Redis requests from clients are serialized as arrays before sending them (* is array length, $ is string length) + /* Redis requests from clients are serialized as arrays before sending them (* is array length, $ is string length) */ test_redis_parse_req_success_case("*2\r\n$3\r\nGET\r\n$3\r\nkey\r\n", MSG_REQ_REDIS_GET); test_redis_parse_req_success_case("*2\r\n$4\r\nMGET\r\n$1\r\nx\r\n", MSG_REQ_REDIS_MGET); test_redis_parse_req_success_case("*3\r\n$4\r\nMGET\r\n$1\r\nx\r\n$10\r\nabcdefghij\r\n", MSG_REQ_REDIS_MGET); test_redis_parse_req_success_case("*3\r\n$3\r\nSET\r\n$10\r\nkey4567890\r\n$5\r\nVALUE\r\n", MSG_REQ_REDIS_SET); - test_redis_parse_req_success_case("*2\r\n$4\r\nLLEN\r\n$6\r\nmylist\r\n", MSG_REQ_REDIS_LLEN); // LLEN command + test_redis_parse_req_success_case("*1\r\n$7\r\nCOMMAND\r\n", MSG_REQ_REDIS_COMMAND); test_redis_parse_req_success_case("*1\r\n$6\r\nLOLWUT\r\n", MSG_REQ_REDIS_LOLWUT); test_redis_parse_req_success_case("*2\r\n$6\r\nLOLWUT\r\n$2\r\n40\r\n", MSG_REQ_REDIS_LOLWUT); + test_redis_parse_req_success_case("*2\r\n$4\r\nLLEN\r\n$6\r\nmylist\r\n", MSG_REQ_REDIS_LLEN); /* LLEN command */ test_redis_parse_req_success_case("*1\r\n$4\r\nPING\r\n", MSG_REQ_REDIS_PING); } -static void test_redis_parse_rsp_success_case(char* data) { +static void test_redis_parse_rsp_success_case(const char* data, int expected) { int original_failures = failures; struct conn fake_client = {0}; struct mbuf *m = mbuf_get(); - const int SW_START = 0; // Same as SW_START in redis_parse_rsp + const int SW_START = 0; /* Same as SW_START in redis_parse_rsp */ struct msg *rsp = msg_get(&fake_client, 0, 1); rsp->state = SW_START; rsp->token = NULL; - const size_t datalen = (int)strlen(data); + const size_t datalen = strlen(data); - // Copy data into the message - mbuf_copy(m, (uint8_t*)data, datalen); - // Insert a single buffer into the message mbuf header + /* Copy data into the message */ + mbuf_copy(m, (const uint8_t*)data, datalen); + /* Insert a single buffer into the message mbuf header */ STAILQ_INIT(&rsp->mhdr); ASSERT(STAILQ_EMPTY(&rsp->mhdr)); mbuf_insert(&rsp->mhdr, m); @@ -112,29 +125,45 @@ static void test_redis_parse_rsp_success_case(char* data) { redis_parse_rsp(rsp); expect_same_ptr(m->last, rsp->pos, "redis_parse_rsp: expected rsp->pos to be m->last"); - expect_same_uint32_t(SW_START, rsp->state, "redis_parse_rsp: expected full buffer to be parsed"); + expect_same_int(SW_START, rsp->state, "redis_parse_rsp: expected full buffer to be parsed"); + expect_same_int(expected, rsp->type, "redis_parse_rsp: expected response type to be parsed"); expect_same_uint32_t(1, rsp->rnarg ? rsp->rnarg : 1, "expected remaining args to be 0 or 1"); msg_put(rsp); - // mbuf_put(m); + /* mbuf_put(m); */ if (failures > original_failures) { fprintf(stderr, "test_redis_parse_rsp_success_case failed for %s", data); } } -// Test support for https://redis.io/topics/protocol +/* Test support for https://redis.io/topics/protocol */ static void test_redis_parse_rsp_success(void) { - test_redis_parse_rsp_success_case("-CUSTOMERR\r\n"); // Error message without a space - test_redis_parse_rsp_success_case("-Error message\r\n"); // Error message - test_redis_parse_rsp_success_case("+OK\r\n"); // Error message without a space - - test_redis_parse_rsp_success_case("$6\r\nfoobar\r\n"); // bulk string - test_redis_parse_rsp_success_case("$0\r\n\r\n"); // empty bulk string - test_redis_parse_rsp_success_case("$-1\r\n"); // null value - test_redis_parse_rsp_success_case("*0\r\n"); // empty array - test_redis_parse_rsp_success_case("*2\r\n$3\r\nfoo\r\n$3\r\nbar\r\n"); // array with 2 bulk strings - test_redis_parse_rsp_success_case("*3\r\n:1\r\n:2\r\n:3\r\n"); // array with 3 integers - test_redis_parse_rsp_success_case("*-1\r\n"); // null array for BLPOP + /* Error message without a space */ + test_redis_parse_rsp_success_case("-CUSTOMERR\r\n", MSG_RSP_REDIS_ERROR); + /* Error message */ + test_redis_parse_rsp_success_case("-Error message\r\n", MSG_RSP_REDIS_ERROR); + /* Error message without a space */ + test_redis_parse_rsp_success_case("+OK\r\n", MSG_RSP_REDIS_STATUS); + /* bulk string */ + test_redis_parse_rsp_success_case("$6\r\nfoobar\r\n", MSG_RSP_REDIS_BULK); + /* empty bulk string */ + test_redis_parse_rsp_success_case("$0\r\n\r\n", MSG_RSP_REDIS_BULK); + /* null value */ + test_redis_parse_rsp_success_case("$-1\r\n", MSG_RSP_REDIS_BULK); + /* empty array */ + test_redis_parse_rsp_success_case("*0\r\n", MSG_RSP_REDIS_MULTIBULK); + /* array with 2 bulk strings */ + test_redis_parse_rsp_success_case("*2\r\n$3\r\nfoo\r\n$3\r\nbar\r\n", + MSG_RSP_REDIS_MULTIBULK); + /* array with 3 integers */ + test_redis_parse_rsp_success_case("*3\r\n:1\r\n:2\r\n:3\r\n", + MSG_RSP_REDIS_MULTIBULK); + /* null array for BLPOP */ + test_redis_parse_rsp_success_case("*-1\r\n", MSG_RSP_REDIS_MULTIBULK); + /* + * Test support for parsing arrays of arrays. + * They can be returned by COMMAND, EVAL, etc. + */ test_redis_parse_rsp_success_case("*2\r\n" "*3\r\n" ":1\r\n" @@ -142,22 +171,22 @@ static void test_redis_parse_rsp_success(void) { ":3\r\n" "*2\r\n" "+Foo\r\n" - "-Bar\r\n"); // array of 2 arrays + "-Bar\r\n", MSG_RSP_REDIS_MULTIBULK); /* array of 2 arrays */ } -static void test_memcache_parse_rsp_success_case(char* data, int expected) { +static void test_memcache_parse_rsp_success_case(const char* data, int expected) { struct conn fake_client = {0}; struct mbuf *m = mbuf_get(); - const int SW_START = 0; // Same as SW_START in memcache_parse_rsp + const int SW_START = 0; /* Same as SW_START in memcache_parse_rsp */ struct msg *rsp = msg_get(&fake_client, 0, 0); rsp->state = SW_START; rsp->token = NULL; - const size_t datalen = (int)strlen(data); + const size_t datalen = strlen(data); - // Copy data into the message - mbuf_copy(m, (uint8_t*)data, datalen); - // Insert a single buffer into the message mbuf header + /* Copy data into the message */ + mbuf_copy(m, (const uint8_t*)data, datalen); + /* Insert a single buffer into the message mbuf header */ STAILQ_INIT(&rsp->mhdr); ASSERT(STAILQ_EMPTY(&rsp->mhdr)); mbuf_insert(&rsp->mhdr, m); @@ -165,12 +194,12 @@ static void test_memcache_parse_rsp_success_case(char* data, int expected) { memcache_parse_rsp(rsp); expect_same_ptr(m->last, rsp->pos, "memcache_parse_rsp: expected rsp->pos to be m->last"); - expect_same_uint32_t(SW_START, rsp->state, "memcache_parse_rsp: expected state to be SW_START after parsing full buffer"); - expect_same_uint32_t(expected, rsp->type, "memcache_parse_rsp: expected response type to be parsed"); - expect_same_uint32_t(0, fake_client.err, "redis_parse_req: expected no connection error"); + expect_same_int(SW_START, rsp->state, "memcache_parse_rsp: expected state to be SW_START after parsing full buffer"); + expect_same_int(expected, rsp->type, "memcache_parse_rsp: expected response type to be parsed"); + expect_same_int(0, fake_client.err, "redis_parse_req: expected no connection error"); msg_put(rsp); - // mbuf_put(m); + /* mbuf_put(m); */ } static void test_memcache_parse_rsp_success(void) { @@ -185,19 +214,19 @@ static void test_memcache_parse_rsp_success(void) { test_memcache_parse_rsp_success_case("VALUE key 0 2\r\nab\r\nVALUE key2 0 2\r\ncd\r\nEND\r\n", MSG_RSP_MC_END); } -static void test_memcache_parse_req_success_case(char* data, int expected) { +static void test_memcache_parse_req_success_case(const char* data, int expected) { struct conn fake_client = {0}; struct mbuf *m = mbuf_get(); - const int SW_START = 0; // Same as SW_START in memcache_parse_req + const int SW_START = 0; /* Same as SW_START in memcache_parse_req */ struct msg *req = msg_get(&fake_client, 1, 0); req->state = SW_START; req->token = NULL; - const size_t datalen = (int)strlen(data); + const size_t datalen = strlen(data); - // Copy data into the message - mbuf_copy(m, (uint8_t*)data, datalen); - // Insert a single buffer into the message mbuf header + /* Copy data into the message */ + mbuf_copy(m, (const uint8_t*)data, datalen); + /* Insert a single buffer into the message mbuf header */ STAILQ_INIT(&req->mhdr); ASSERT(STAILQ_EMPTY(&req->mhdr)); mbuf_insert(&req->mhdr, m); @@ -205,12 +234,12 @@ static void test_memcache_parse_req_success_case(char* data, int expected) { memcache_parse_req(req); expect_same_ptr(m->last, req->pos, "memcache_parse_req: expected req->pos to be m->last"); - expect_same_uint32_t(SW_START, req->state, "memcache_parse_req: expected state to be SW_START after parsing full buffer"); - expect_same_uint32_t(expected, req->type, "memcache_parse_req: expected response type to be parsed"); - expect_same_uint32_t(0, fake_client.err, "redis_parse_req: expected no connection error"); + expect_same_int(SW_START, req->state, "memcache_parse_req: expected state to be SW_START after parsing full buffer"); + expect_same_int(expected, req->type, "memcache_parse_req: expected response type to be parsed"); + expect_same_int(0, fake_client.err, "redis_parse_req: expected no connection error"); msg_put(req); - // mbuf_put(m); + /* mbuf_put(m); */ } static void test_memcache_parse_req_success(void) { diff --git a/tests/test_redis/test_basic.py b/tests/test_redis/test_basic.py index 58ecacf8..632dfd6d 100644 --- a/tests/test_redis/test_basic.py +++ b/tests/test_redis/test_basic.py @@ -140,6 +140,7 @@ def test_issue_323(): c = getconn() assert_equal([1, b'OK'], c.eval("return {1, redis.call('set', 'x', '1')}", 1, 'tmp')) + # Test processing deeply nested multibulk responses assert_equal([[[[[[[[[[[[[[[[[[[[b'value']]]]]]]]]]]]]]]]]]], b'other'], c.eval("return {{{{{{{{{{{{{{{{{{{{'value'}}}}}}}}}}}}}}}}}}}, 'other'}", 1, 'tmp')) def setup_and_wait(): diff --git a/travis.sh b/travis.sh index fe95f06e..c0fd36f8 100755 --- a/travis.sh +++ b/travis.sh @@ -51,7 +51,7 @@ docker run \ -e REDIS_VER=$REDIS_VER \ --name=$DOCKER_IMG_NAME \ $DOCKER_TAG \ - nosetests -v test_redis test_memcache test_system.test_sentinel + nosetests -v test_redis test_memcache test_system if [ $UNIT_TEST_FAIL = yes ]; then echo "See earlier output, unit tests failed" 1>&2