Message from discussion
Regressions in 2.6 when out of memory & writing during multi-exec?
Received: by 10.50.33.180 with SMTP id s20mr3093399igi.5.1353070480253;
Fri, 16 Nov 2012 04:54:40 -0800 (PST)
X-BeenThere: redis-db@googlegroups.com
Received: by 10.50.108.197 with SMTP id hm5ls834851igb.7.gmail; Fri, 16 Nov
2012 04:54:34 -0800 (PST)
Received: by 10.50.173.68 with SMTP id bi4mr3137462igc.1.1353070474707;
Fri, 16 Nov 2012 04:54:34 -0800 (PST)
Received: by 10.50.173.68 with SMTP id bi4mr3137460igc.1.1353070474681;
Fri, 16 Nov 2012 04:54:34 -0800 (PST)
Return-Path: <mar...@new-bamboo.co.uk>
Received: from mail-ie0-f169.google.com (mail-ie0-f169.google.com [209.85.223.169])
by gmr-mx.google.com with ESMTPS id wu4si43508igb.3.2012.11.16.04.54.34
(version=TLSv1/SSLv3 cipher=OTHER);
Fri, 16 Nov 2012 04:54:34 -0800 (PST)
Received-SPF: neutral (google.com: 209.85.223.169 is neither permitted nor denied by best guess record for domain of mar...@new-bamboo.co.uk) client-ip=209.85.223.169;
Authentication-Results: gmr-mx.google.com; spf=neutral (google.com: 209.85.223.169 is neither permitted nor denied by best guess record for domain of mar...@new-bamboo.co.uk) smtp.mail=mar...@new-bamboo.co.uk
Received: by mail-ie0-f169.google.com with SMTP id 10so3353083ied.28
for <redis-db@googlegroups.com>; Fri, 16 Nov 2012 04:54:34 -0800 (PST)
d=google.com; s=20120113;
h=mime-version:in-reply-to:references:from:date:message-id:subject:to
:content-type:x-gm-message-state;
bh=r0fYyTeku3Sy9F7KIp5liSdMe9zYWg56w5wyYtCxazE=;
b=ofZ0XtK3GBQMrsvAQRvHtXElOcxIY3JTQDoxGKjA9cw7C9bFaZ3rG/+mTkmkhQG8JY
2T27xul6atsw9AHD26gvr9zBxZK3njWRDdEcmA783grEntHd8HAjmkjVb0bmmREHfWms
a5sxPb/ARHAbs4EMnWIOSzITNOyqZ1II/xAp7rzWi7nlbCqo6grjTQI7GZbybDv4nf1t
AR9GkUoyKnjvFFilPm/pppuyA7gbFOjpdXRbzy02SBIDbcL1XCkQneoj+dVnNQCNbFWA
SlT9CksEFJa+Px1Yp6mCMv/acUUE4EtQEvBArkgdymp8eNO6CL3l/nepPE6ba2sFhNW2
QFtg==
Received: by 10.42.68.203 with SMTP id y11mr3588429ici.26.1353070474320; Fri,
16 Nov 2012 04:54:34 -0800 (PST)
MIME-Version: 1.0
Received: by 10.50.57.168 with HTTP; Fri, 16 Nov 2012 04:54:14 -0800 (PST)
In-Reply-To: <CA+XzkVecAtRXoDVSQ0YbZogZp+AuNskOOeGRyNdTpHnOpGe...@mail.gmail.com>
References: <CAP0aOtoTMd8qY0+XENa9mbfWNtr1R2BkcNuDYOZ=OC-y34u...@mail.gmail.com>
<CA+XzkVecAtRXoDVSQ0YbZogZp+AuNskOOeGRyNdTpHnOpGe...@mail.gmail.com>
From: Martyn Loughran <mar...@pusher.com>
Date: Fri, 16 Nov 2012 12:54:14 +0000
Message-ID: <CAP0aOtqJ7=P3oyqFuDw2rnz8328YH8kZo+V95UYOB4QALyN...@mail.gmail.com>
Subject: Re: Regressions in 2.6 when out of memory & writing during multi-exec?
To: redis-db@googlegroups.com
Content-Type: multipart/alternative; boundary=20cf30334b5ffa5a5504ce9c44dc
X-Gm-Message-State: ALoCoQn/7p7LFQxmVzcLO/PAeW/Y1RLtvwMstdnvlbsi4r1XB9OO4Z7YovET6k9ub2x43AP+1dU/
--20cf30334b5ffa5a5504ce9c44dc
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: quoted-printable
Thanks for dealing with this so quickly Salvatore!
I completely agree with your monologue last night, seems very sensible :)
Specifically having the exec return an error seems correct, and makes
clients easy to write. I don't see any problems with discarding the
transaction; presumably clients which abstract transactions currently have
no option but to discard after a failed exec.
I've tried the branch and it works perfectly.
Thanks again,
Martyn
On 15 November 2012 19:21, Salvatore Sanfilippo <anti...@gmail.com> wrote:
> Ok I've an implementation to share with you:
>
> https://github.com/antirez/redis/compare/safer-exec
>
> This is the behavior in short:
>
> 1) EXEC fails if at least one command was not queued because of an error.
> 2) A generic error is always reported on EXEC.
>
> Example:
>
> redis 127.0.0.1:6379> multi
> OK
> redis 127.0.0.1:6379> set key1 value1
> QUEUED
>
> (config set maxmemory 1)
>
> redis 127.0.0.1:6379> set key2 value2
> (error) OOM command not allowed when used memory > 'maxmemory'.
>
> (config set maxmemory 0)
>
> redis 127.0.0.1:6379> set key3 value3
> QUEUED
> redis 127.0.0.1:6379> EXEC
> (error) EXECABORT Transaction aborted, errors detected while queueing
> commands.'.
>
> I think that's the best bet for us IMHO, because centralizing the
> error reporting only on EXEC requires accumulating the errors, instead
> this way we get a generic error with EXEC, but when clients detect
> this error (-EXECABORT) they should also report the other erros
> accordingly.
>
> Other clients not using pipelining may or may not report errors ASAP
> as commands are queued.
>
> Basically it's up to the client designer, but Redis tries to play
> nicely at this point.
>
> Only problem, the new behavior is to DISCARD on EXEC error, that makes
> a lot of sense IMHO.
>
> Client developers, please raise your hand if this breaks your client!
> Thanks.
> This fix should be backported into 2.6 so it is important that it
> works as expected with existing clients.
>
> Cheers,
> Salvatore
>
> On Thu, Nov 15, 2012 at 6:56 PM, Martyn Loughran <mar...@pusher.com>
> wrote:
> > While upgrading from 2.4 to 2.6 I noticed some changes which I haven't
> seen
> > documented, and look like regressions to me.
> >
> > Scenario: out of memory & writing during multi-exec
> >
> > # Change 1 - exec succeeds even if one of commands was an error
> >
> > On redis 2.4 (tested 2.4.17)
> >
> > redis 127.0.0.1:6379> config get maxmemory
> > 1) "maxmemory"
> > 2) "0"
> > redis 127.0.0.1:6379> multi
> > OK
> > redis 127.0.0.1:6379> set foo yyy
> > QUEUED
> >
> > In another client set memory limit `config set maxmemory 1`
> >
> > redis 127.0.0.1:6379> set bar uuu
> > (error) ERR command not allowed when used memory > 'maxmemory'
> > redis 127.0.0.1:6379> exec
> > (error) ERR command not allowed when used memory > 'maxmemory'
> >
> > Transaction still open, need to call discard
> >
> > On redis 2.6 (tested 2.6.4)
> >
> > redis 127.0.0.1:6379> config get maxmemory
> > 1) "maxmemory"
> > 2) "0"
> > redis 127.0.0.1:6379> multi
> > OK
> > redis 127.0.0.1:6379> set foo yyy
> > QUEUED
> >
> > In another client set memory limit `config set maxmemory 1`
> >
> > redis 127.0.0.1:6379> set bar uuu
> > (error) OOM command not allowed when used memory > 'maxmemory'.
> > redis 127.0.0.1:6379> exec
> > 1) OK
> > redis 127.0.0.1:6379> get foo
> > "yyy"
> > redis 127.0.0.1:6379> get bar
> > (nil)
> >
> > This seems like a pretty serious issue. foo and bar were set during a
> > transaction, but one one of the keys was written!
> >
> > The only way to code around this is to check the response from each of
> the
> > commands during a multi, and to only exec if there were no errors. This
> is
> > problematic for all sorts of reasons. Previously it was easy to see
> whether
> > the transaction succeeded by reading the exec reply.
> >
> > The OOM error is also inconsistent with this line in the transaction do=
cs
> > "When a Redis connection is in the context of a MULTI request, all
> commands
> > will reply with the string QUEUED unless they are syntactically
> incorrect".
> >
> > # Change 2 - memory limit enforced during queuing rather than exec
> >
> > This is related to the first change, but I just wanted to be explicit
> >
> > On redis 2.4 (tested 2.4.17) the memory limit is enforced when exec is
> > called
> >
> > redis 127.0.0.1:6379> multi
> > OK
> > redis 127.0.0.1:6379> set foo asdf
> > QUEUED
> >
> > In another client set memory limit `config set maxmemory 1`, then exec
> the
> > transaction
> >
> > redis 127.0.0.1:6379> exec
> > (error) ERR command not allowed when used memory > 'maxmemory'
> >
> > As expected the set fails.
> >
> > For redis 2.6 (tested 2.6.4), the set does not fail
> >
> > redis 127.0.0.1:6379> multi
> > OK
> > redis 127.0.0.1:6379> set foo asdf
> > QUEUED
> >
> > In another client set memory limit `config set maxmemory 1`, then exec
> the
> > transaction
> >
> > redis 127.0.0.1:6379> exec
> > 1) OK
> >
> > This feels like a bug to me.
> >
> > Thanks very much for all your work on redis guys :)
> >
> > Martyn
> >
> > --
> > You received this message because you are subscribed to the Google Grou=
ps
> > "Redis DB" group.
> > To post to this group, send email to redis-db@googlegroups.com.
> > To unsubscribe from this group, send email to
> > redis-db+unsubscribe@googlegroups.com.
> > For more options, visit this group at
> > http://groups.google.com/group/redis-db?hl=3Den.
>
>
>
> --
> Salvatore 'antirez' Sanfilippo
> open source developer - VMware
> http://invece.org
>
> Beauty is more important in computing than anywhere else in technology
> because software is so complicated. Beauty is the ultimate defence
> against complexity.
> =97 David Gelernter
>
> --
> You received this message because you are subscribed to the Google Groups
> "Redis DB" group.
> To post to this group, send email to redis-db@googlegroups.com.
> To unsubscribe from this group, send email to
> redis-db+unsubscribe@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/redis-db?hl=3Den.
>
>
--20cf30334b5ffa5a5504ce9c44dc
Content-Type: text/html; charset=windows-1252
Content-Transfer-Encoding: quoted-printable
<div>Thanks for dealing with this so quickly Salvatore!</div><div><br></div=
><div>I completely agree with your monologue last night, seems very sensibl=
e :) Specifically having the exec return an error seems correct, and makes =
clients easy to write. I don't see any problems with discarding the tra=
nsaction; presumably clients which abstract transactions currently have no =
option but to discard after a failed exec.</div>
<div><br></div><div>I've tried the branch and it works perfectly.</div>=
<div><br></div><div>Thanks again,</div><div>Martyn</div><div class=3D"gmail=
_extra"><br><br><div class=3D"gmail_quote">On 15 November 2012 19:21, Salva=
tore Sanfilippo <span dir=3D"ltr"><<a href=3D"mailto:anti...@gmail.com" =
target=3D"_blank">anti...@gmail.com</a>></span> wrote:<br>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex">Ok I've an implementation to share with =
you:<br>
<br>
<a href=3D"https://github.com/antirez/redis/compare/safer-exec" target=3D"_=
blank">https://github.com/antirez/redis/compare/safer-exec</a><br>
<br>
This is the behavior in short:<br>
<br>
1) EXEC fails if at least one command was not queued because of an error.<b=
r>
2) A generic error is always reported on EXEC.<br>
<br>
Example:<br>
<div class=3D"im"><br>
redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.0.0.1:6379</a=
>> multi<br>
OK<br>
</div>redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.0.0.1:6=
379</a>> set key1 value1<br>
QUEUED<br>
<br>
(config set maxmemory 1)<br>
<br>
redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.0.0.1:6379</a=
>> set key2 value2<br>
<div class=3D"im">(error) OOM command not allowed when used memory > =
9;maxmemory'.<br>
<br>
</div>(config set maxmemory 0)<br>
<br>
redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.0.0.1:6379</a=
>> set key3 value3<br>
QUEUED<br>
redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.0.0.1:6379</a=
>> EXEC<br>
(error) EXECABORT Transaction aborted, errors detected while queueing<br>
commands.'.<br>
<br>
I think that's the best bet for us IMHO, because centralizing the<br>
error reporting only on EXEC requires accumulating the errors, instead<br>
this way we get a generic error with EXEC, but when clients detect<br>
this error (-EXECABORT) they should also report the other erros<br>
accordingly.<br>
<br>
Other clients not using pipelining may or may not report errors ASAP<br>
as commands are queued.<br>
<br>
Basically it's up to the client designer, but Redis tries to play<br>
nicely at this point.<br>
<br>
Only problem, the new behavior is to DISCARD on EXEC error, that makes<br>
a lot of sense IMHO.<br>
<br>
Client developers, please raise your hand if this breaks your client! Thank=
s.<br>
This fix should be backported into 2.6 so it is important that it<br>
works as expected with existing clients.<br>
<div class=3D"im HOEnZb"><br>
Cheers,<br>
Salvatore<br>
<br>
On Thu, Nov 15, 2012 at 6:56 PM, Martyn Loughran <<a href=3D"mailto:mart=
y...@pusher.com">mar...@pusher.com</a>> wrote:<br>
</div><div class=3D"HOEnZb"><div class=3D"h5">> While upgrading from 2.4=
to 2.6 I noticed some changes which I haven't seen<br>
> documented, and look like regressions to me.<br>
><br>
> Scenario: out of memory & writing during multi-exec<br>
><br>
> # Change 1 - exec succeeds even if one of commands was an error<br>
><br>
> On redis 2.4 (tested 2.4.17)<br>
><br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> config get maxmemory<br>
> =A0 =A0 1) "maxmemory"<br>
> =A0 =A0 2) "0"<br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> multi<br>
> =A0 =A0 OK<br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> set foo yyy<br>
> =A0 =A0 QUEUED<br>
><br>
> In another client set memory limit `config set maxmemory 1`<br>
><br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> set bar uuu<br>
> =A0 =A0 (error) ERR command not allowed when used memory > 'max=
memory'<br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> exec<br>
> =A0 =A0 (error) ERR command not allowed when used memory > 'max=
memory'<br>
><br>
> =A0 =A0 Transaction still open, need to call discard<br>
><br>
> On redis 2.6 (tested 2.6.4)<br>
><br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> config get maxmemory<br>
> =A0 =A0 1) "maxmemory"<br>
> =A0 =A0 2) "0"<br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> multi<br>
> =A0 =A0 OK<br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> set foo yyy<br>
> =A0 =A0 QUEUED<br>
><br>
> In another client set memory limit `config set maxmemory 1`<br>
><br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> set bar uuu<br>
> =A0 =A0 (error) OOM command not allowed when used memory > 'max=
memory'.<br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> exec<br>
> =A0 =A0 1) OK<br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> get foo<br>
> =A0 =A0 "yyy"<br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> get bar<br>
> =A0 =A0 (nil)<br>
><br>
> This seems like a pretty serious issue. foo and bar were set during a<=
br>
> transaction, but one one of the keys was written!<br>
><br>
> The only way to code around this is to check the response from each of=
the<br>
> commands during a multi, and to only exec if there were no errors. Thi=
s is<br>
> problematic for all sorts of reasons. Previously it was easy to see wh=
ether<br>
> the transaction succeeded by reading the exec reply.<br>
><br>
> The OOM error is also inconsistent with this line in the transaction d=
ocs<br>
> "When a Redis connection is in the context of a MULTI request, al=
l commands<br>
> will reply with the string QUEUED unless they are syntactically incorr=
ect".<br>
><br>
> # Change 2 - memory limit enforced during queuing rather than exec<br>
><br>
> This is related to the first change, but I just wanted to be explicit<=
br>
><br>
> On redis 2.4 (tested 2.4.17) the memory limit is enforced when exec is=
<br>
> called<br>
><br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> multi<br>
> =A0 =A0 OK<br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> set foo asdf<br>
> =A0 =A0 QUEUED<br>
><br>
> In another client set memory limit `config set maxmemory 1`, then exec=
the<br>
> transaction<br>
><br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> exec<br>
> =A0 =A0 (error) ERR command not allowed when used memory > 'max=
memory'<br>
><br>
> As expected the set fails.<br>
><br>
> For redis 2.6 (tested 2.6.4), the set does not fail<br>
><br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> multi<br>
> =A0 =A0 OK<br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> set foo asdf<br>
> =A0 =A0 QUEUED<br>
><br>
> In another client set memory limit `config set maxmemory 1`, then exec=
the<br>
> transaction<br>
><br>
> =A0 =A0 redis <a href=3D"http://127.0.0.1:6379" target=3D"_blank">127.=
0.0.1:6379</a>> exec<br>
> =A0 =A0 1) OK<br>
><br>
> This feels like a bug to me.<br>
><br>
> Thanks very much for all your work on redis guys :)<br>
><br>
> Martyn<br>
><br>
</div></div><div class=3D"im HOEnZb">> --<br>
> You received this message because you are subscribed to the Google Gro=
ups<br>
> "Redis DB" group.<br>
> To post to this group, send email to <a href=3D"mailto:redis-db@google=
groups.com">redis-db@googlegroups.com</a>.<br>
> To unsubscribe from this group, send email to<br>
> <a href=3D"mailto:redis-db%2Bunsubscribe@googlegroups.com">redis-db+un=
subscribe@googlegroups.com</a>.<br>
> For more options, visit this group at<br>
> <a href=3D"http://groups.google.com/group/redis-db?hl=3Den" target=3D"=
_blank">http://groups.google.com/group/redis-db?hl=3Den</a>.<br>
<br>
<br>
<br>
</div><div class=3D"im HOEnZb">--<br>
Salvatore 'antirez' Sanfilippo<br>
open source developer - VMware<br>
<a href=3D"http://invece.org" target=3D"_blank">http://invece.org</a><br>
<br>
Beauty is more important in computing than anywhere else in technology<br>
because software is so complicated. Beauty is the ultimate defence<br>
against complexity.<br>
=A0 =A0 =A0 =A0=97 David Gelernter<br>
<br>
</div><div class=3D"HOEnZb"><div class=3D"h5">--<br>
You received this message because you are subscribed to the Google Groups &=
quot;Redis DB" group.<br>
To post to this group, send email to <a href=3D"mailto:redis-db@googlegroup=
s.com">redis-db@googlegroups.com</a>.<br>
To unsubscribe from this group, send email to <a href=3D"mailto:redis-db%2B=
unsubscribe@googlegroups.com">redis-db+unsubscribe@googlegroups.com</a>.<br=
>
For more options, visit this group at <a href=3D"http://groups.google.com/g=
roup/redis-db?hl=3Den" target=3D"_blank">http://groups.google.com/group/red=
is-db?hl=3Den</a>.<br>
<br>
</div></div></blockquote></div><br></div>
--20cf30334b5ffa5a5504ce9c44dc--