You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by GitBox <gi...@apache.org> on 2022/07/03 09:43:57 UTC
[GitHub] [incubator-kvrocks] mapleFU opened a new pull request, #709: Enhancement: not call GetCurrentTime when possible
mapleFU opened a new pull request, #709:
URL: https://github.com/apache/incubator-kvrocks/pull/709
Note: this merge request will regard `expire <= 0` as "not has expire", maybe break the constraint before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #709: Enhancement: not call GetCurrentTime when possible
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #709:
URL: https://github.com/apache/incubator-kvrocks/pull/709#issuecomment-1173053971
> > ASAN:DEADLYSIGNAL
> > =================================================================
> > ==30388==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000000f0 (pc 0x55c5e3f1823b bp 0x7fb31cbe58b0 sp 0x7fb31cbe5888 T103)
> > ==30388==The signal is caused by a READ memory access.
> > ==30388==Hint: address points to the zero page.
>
> emmm, does this caused by my commits? No idea why this occurs
Yes, this failure isn't related to current PR, will record the stack and rerun CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on pull request #709: Enhancement: not call GetCurrentTime when possible
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #709:
URL: https://github.com/apache/incubator-kvrocks/pull/709#issuecomment-1173314999
> This mr solves issue https://github.com/apache/incubator-kvrocks/pull/709
@mapleFU there is no "issue" #709 .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] mapleFU commented on pull request #709: Enhancement: not call GetCurrentTime when possible
Posted by GitBox <gi...@apache.org>.
mapleFU commented on PR #709:
URL: https://github.com/apache/incubator-kvrocks/pull/709#issuecomment-1174603073
I think it's from redis: https://redis.io/commands/ttl/ @tisonkun
We can use a enum to represent them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] mapleFU commented on pull request #709: Enhancement: not call GetCurrentTime when possible
Posted by GitBox <gi...@apache.org>.
mapleFU commented on PR #709:
URL: https://github.com/apache/incubator-kvrocks/pull/709#issuecomment-1173051830
> ASAN:DEADLYSIGNAL
=================================================================
==30388==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000000f0 (pc 0x55c5e3f1823b bp 0x7fb31cbe58b0 sp 0x7fb31cbe5888 T103)
==30388==The signal is caused by a READ memory access.
==30388==Hint: address points to the zero page.
emmm, does this caused by my commits? No idea why this occurs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] mapleFU commented on pull request #709: Enhancement: not call GetCurrentTime when possible
Posted by GitBox <gi...@apache.org>.
mapleFU commented on PR #709:
URL: https://github.com/apache/incubator-kvrocks/pull/709#issuecomment-1173060406
> Yes, this failure isn't related to current PR, will record the stack and rerun CI.
It might be some concurrency problems on shutdown, since `0x0000000000f0` is a strange address.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] mapleFU commented on pull request #709: Enhancement: not call GetCurrentTime when possible
Posted by GitBox <gi...@apache.org>.
mapleFU commented on PR #709:
URL: https://github.com/apache/incubator-kvrocks/pull/709#issuecomment-1173464210
> @mapleFU there is no "issue" #709 .
So sorry for making the mistake, I have update it to https://github.com/apache/incubator-kvrocks/issues/708. PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #709: Enhancement: not call GetCurrentTime when possible
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #709:
URL: https://github.com/apache/incubator-kvrocks/pull/709#discussion_r913377935
##########
src/redis_metadata.cc:
##########
@@ -232,11 +232,14 @@ RedisType Metadata::Type() const {
int32_t Metadata::TTL() const {
int64_t now;
+ if (expire <= 0) {
+ return -1;
+ }
Review Comment:
https://github.com/apache/incubator-kvrocks/blob/unstable/src/redis_db.cc#L146
one comment explained `-2` here, far away from this function though :rofl:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #709: Enhancement: not call GetCurrentTime when possible
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #709:
URL: https://github.com/apache/incubator-kvrocks/pull/709#issuecomment-1174739307
Thanks all, merging...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on a diff in pull request #709: Enhancement: not call GetCurrentTime when possible
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #709:
URL: https://github.com/apache/incubator-kvrocks/pull/709#discussion_r913371833
##########
src/redis_metadata.cc:
##########
@@ -232,11 +232,14 @@ RedisType Metadata::Type() const {
int32_t Metadata::TTL() const {
int64_t now;
+ if (expire <= 0) {
+ return -1;
+ }
Review Comment:
What's the meaning of `-1` and `-2` returns here? As @mapleFU described here change the result `expire <0 && expire < now` from `-2` to `-1`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk merged pull request #709: Enhancement: not call GetCurrentTime when possible
Posted by GitBox <gi...@apache.org>.
git-hulk merged PR #709:
URL: https://github.com/apache/incubator-kvrocks/pull/709
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org