- 02 May, 2017 1 commit
-
-
antirez authored
-
- 28 Apr, 2017 2 commits
-
-
antirez authored
-
antirez authored
Experimentally verified that it can trigger the issue reverting the fix. At least on my system... Being the bug time/backlog dependant, it is very hard to tell if this test will be able to trigger the problem consistently, however even if it triggers the problem once in a while, we'll see it in the CI environment at http://ci.redis.io.
-
- 27 Apr, 2017 1 commit
-
-
antirez authored
The master client cleanup was incomplete: resetClient() was missing and the output buffer of the client was not reset, so pending commands related to the previous connection could be still sent. The first problem caused the client argument vector to be, at times, half populated, so that when the correct replication stream arrived the protcol got mixed to the arugments creating invalid commands that nobody called. Thanks to @yangsiran for also investigating this problem, after already providing important design / implementation hints for the original PSYNC2 issues (see referenced Github issue). Note that this commit adds a new function to the list library of Redis in order to be able to reset a list without destroying it. Related to issue #3899.
-
- 22 Apr, 2017 4 commits
-
-
antirez authored
Related to #3786.
-
antirez authored
Apparently 1.4 is too low compared to what you get in certain setups (including mine). I raised it to 1.55 that hopefully is still enough to test that the fragmentation went down from 1.7 but without incurring in issues, however the test setup may be still fragile so certain times this may lead to false positives again, it's hard to test for these things in a determinsitic way. Related to #3786.
-
oranagra authored
-
- 21 Apr, 2017 1 commit
-
-
antirez authored
Normally we never check for OOM conditions inside Redis since the allocator will always return a pointer or abort the program on OOM conditons. However we cannot have control on epool_create(), that may fail for kernel OOM (according to the manual page) even if all the parameters are correct, so the function aeCreateEventLoop() may indeed return NULL and this condition must be checked.
-
- 20 Apr, 2017 1 commit
-
-
Salvatore Sanfilippo authored
update block->free after some diff data are written to the child process
-
- 19 Apr, 2017 3 commits
-
-
antirez authored
Close #3940.
-
antirez authored
During the review of the fix for #3899, @yangsiran identified an implementation bug: given that the offset is now relative to the applied part of the replication log, when we cache a master, the successive PSYNC2 request will be made in order to *include* the transaction that was not completely processed. This means that we need to discard any pending transaction from our replication buffer: it will be re-executed.
-
antirez authored
This bug was discovered by @kevinmcgehee and constituted a major hidden bug in the PSYNC2 implementation, caused by the propagation from the master of incomplete commands to slaves. The bug had several results: 1. Borrowing from Kevin text in the issue: "Given that slaves blindly copy over their master's input into their own replication backlog over successive read syscalls, it's possible that with large commands or small TCP buffers, partial commands are present in this buffer. If the master were to fail before successfully propagating the entire command to a slave, the slaves will never execute the partial command (since the client is invalidated) but will copy it to replication backlog which may relay those invalid bytes to its slaves on PSYNC2, corrupting the backlog and possibly other valid commands that follow the failover. Simple command boundaries aren't sufficient to capture this, either, because in the case of a MULTI/EXEC block, if the master successfully propagates a subset of the commands but not the EXEC, then the transaction in the backlog becomes corrupt and could corrupt other slaves that consume this data." 2. As identified by @yangsiran later, there is another effect of the bug. For the same mechanism of the first problem, a slave having another slave, could receive a full resynchronization request with an already half-applied command in the backlog. Once the RDB is ready, it will be sent to the slave, and the replication will continue sending to the sub-slave the other half of the command, which is not valid. The fix, designed by @yangsiran and @antirez, and implemented by @antirez, uses a secondary buffer in order to feed the sub-masters and update the replication backlog and offsets, only when a given part of the query buffer is actually *applied* to the state of the instance, that is, when the command gets processed and the command is not pending in the Redis transaction buffer because of CLIENT_MULTI state. Given that now the backlog and offsets representation are in agreement with the actual processed commands, both issue 1 and 2 should no longer be possible. Thanks to @kevinmcgehee, @yangsiran and @oranagra for their work in identifying and designing a fix for this problem.
-
- 18 Apr, 2017 8 commits
-
-
Salvatore Sanfilippo authored
Reorder to make dict-benchmark compile on Linux
-
antirez authored
-
antirez authored
-
antirez authored
-
Salvatore Sanfilippo authored
fix #3847: add close socket before return ANET_ERR.
-
张文康 authored
-
antirez authored
Also get rid of variables that are now kinda redundant, since the dictionary iterator was removed. This is related to PR #3949.
-
Salvatore Sanfilippo authored
rdb: saving skiplist in reversed order to accelerate the deserialisation process
-
- 17 Apr, 2017 2 commits
-
-
Jan-Erik Rediger authored
Fixes #3944
-
spinlock authored
-
- 15 Apr, 2017 1 commit
-
-
antirez authored
However we allow for 500 milliseconds of tolerance, in order to avoid often discarding semantically valid info (the node is up) because of natural few milliseconds desync among servers even when NTP is used. Note that anyway we should ping the node from time to time regardless and discover if it's actually down from our point of view, since no update is accepted while we have an active ping on the node. Related to #3929.
-
- 14 Apr, 2017 5 commits
-
-
antirez authored
And many other related Github issues... all reporting the same problem. There was probably just not enough backlog in certain unlucky runs. I'll ask people that can reporduce if they see now this as fixed as well.
-
antirez authored
To rely on the fact that nodes in PFAIL state will be shared around by randomly adding them in the gossip section is a weak assumption, especially after changes related to sending less ping/pong packets. We want to always include gossip entries for all the nodes that are in PFAIL state, so that the PFAIL -> FAIL state promotion can happen much faster and reliably. Related to #3929.
-
antirez authored
The gossip section times are 32 bit, so cannot store the milliseconds time but just the seconds approximation, which is good enough for our uses. At the same time however, when comparing the gossip section times of other nodes with our node's view, we need to convert back to milliseconds. Related to #3929. Without this change the patch to reduce the traffic in the bus message does not work.
-
antirez authored
-
antirez authored
Cluster of bigger sizes tend to have a lot of traffic in the cluster bus just for failure detection: a node will try to get a ping reply from another node no longer than when the half the node timeout would elapsed, in order to avoid a false positive. However this means that if we have N nodes and the node timeout is set to, for instance M seconds, we'll have to ping N nodes every M/2 seconds. This N*M/2 pings will receive the same number of pongs, so a total of N*M packets per node. However given that we have a total of N nodes doing this, the total number of messages will be N*N*M. In a 100 nodes cluster with a timeout of 60 seconds, this translates to a total of 100*100*30 packets per second, summing all the packets exchanged by all the nodes. This is, as you can guess, a lot... So this patch changes the implementation in a very simple way in order to trust the reports of other nodes: if a node A reports a node B as alive at least up to a given time, we update our view accordingly. The problem with this approach is that it could result into a subset of nodes being able to reach a given node X, and preventing others from detecting that is actually not reachable from the majority of nodes. So the above algorithm is refined by trusting other nodes only if we do not have currently a ping pending for the node X, and if there are no failure reports for that node. Since each node, anyway, pings 10 other nodes every second (one node every 100 milliseconds), anyway eventually even trusting the other nodes reports, we will detect if a given node is down from our POV. Now to understand the number of packets that the cluster would exchange for failure detection with the patch, we can start considering the random PINGs that the cluster sent anyway as base line: Each node sends 10 packets per second, so the total traffic if no additioal packets would be sent, including PONG packets, would be: Total messages per second = N*10*2 However by trusting other nodes gossip sections will not AWALYS prevent pinging nodes for the "half timeout reached" rule all the times. The math involved in computing the actual rate as N and M change is quite complex and depends also on another parameter, which is the number of entries in the gossip section of PING and PONG packets. However it is possible to compare what happens in cluster of different sizes experimentally. After applying this patch a very important reduction in the number of packets exchanged is trivial to observe, without apparent impacts on the failure detection performances. Actual numbers with different cluster sizes should be published in the Reids Cluster documentation in the future. Related to #3929.
-
- 13 Apr, 2017 1 commit
-
-
antirez authored
First step in order to change Cluster in order to use less messages. Related to issue #3929.
-
- 12 Apr, 2017 2 commits
- 11 Apr, 2017 5 commits
-
-
antirez authored
Otherwise, as it was, it will overwrite whatever the user set. Close #3703.
-
antirez authored
-
antirez authored
-
antirez authored
Close #3927.
-
Salvatore Sanfilippo authored
Expire: Update comment of activeExpireCycle function
-
- 10 Apr, 2017 3 commits
-
-
antirez authored
-
Salvatore Sanfilippo authored
fixed free of blocked client before refering to it
-
antirez authored
If a thread unblocks a client blocked in a module command, by using the RedisMdoule_UnblockClient() API, the event loop may not be awaken until the next timeout of the multiplexing API or the next unrelated I/O operation on other clients. We actually want the client to be served ASAP, so a mechanism is needed in order for the unblocking API to inform Redis that there is a client to serve ASAP. This commit fixes the issue using the old trick of the pipe: when a client needs to be unblocked, a byte is written in a pipe. When we run the list of clients blocked in modules, we consume all the bytes written in the pipe. Writes and reads are performed inside the context of the mutex, so no race is possible in which we consume the bytes that are actually related to an awake request for a client that should still be put into the list of clients to unblock. It was verified that after the fix the server handles the blocked clients with the expected short delay. Thanks to @dvirsky for understanding there was such a problem and reporting it.
-