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/10/13 05:10:36 UTC
[GitHub] [incubator-kvrocks] tisonkun opened a new issue, #983: Flaky test on expire precision
tisonkun opened a new issue, #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983
### Search before asking
- [X] I had searched in the [issues](https://github.com/apache/incubator-kvrocks/issues) and found no similar issues.
### Version
https://github.com/apache/incubator-kvrocks/pull/981/commits/461d0e8cc95cf959df50db2d75fecffbfd919875
### Minimal reproduce step
Flaky test on CI env.
### What did you expect to see?
Tests passed.
### What did you see instead?
```
--- FAIL: TestString (12.24s)
--- FAIL: TestString/GETEX_EXAT_option (0.00s)
assertions.go:36:
Error Trace: /home/runner/work/incubator-kvrocks/incubator-kvrocks/tests/gocase/unit/type/strings/assertions.go:36
/home/runner/work/incubator-kvrocks/incubator-kvrocks/tests/gocase/unit/type/strings/strings_test.go:155
Error: "11s" is not less than or equal to "10s"
Test: TestString/GETEX_EXAT_option
--- FAIL: TestString/GETEX_PXAT_option (0.00s)
assertions.go:36:
Error Trace: /home/runner/work/incubator-kvrocks/incubator-kvrocks/tests/gocase/unit/type/strings/assertions.go:36
/home/runner/work/incubator-kvrocks/incubator-kvrocks/tests/gocase/unit/type/strings/strings_test.go:162
Error: "11s" is not less than or equal to "10s"
Test: TestString/GETEX_PXAT_option
```
### Anything Else?
cc @git-hulk please take a look.
### Are you willing to submit a PR?
- [ ] I'm willing to submit a PR!
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] mapleFU commented on issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
mapleFU commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289317000
@tanruixiang @git-hulk Maybe I made a mistake, after go through go's `time.Now`, the `nsecs` just return a wall time if possible. Let's just waiting for testing now
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
git-hulk commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1277039658
@tisonkun Let me fix it
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289146440
I got this point. But I recommand we solve the problem after (or in) #1032, since ParseTTL changes hugely in this PR. :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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
git-hulk commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289198195
:333333, let's increase the upper boundary.
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
git-hulk commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289915397
> `ck`, it uses `CLOCK_REALTIME`. `CLOCK_REALTIME` can be affected by NTP or setting, so it can move backward.
Yes, I copied the assemble in https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1288812732, Go runtime will use `CLOCK_REALTIME` first then fallback to `CLOCK_MONOTONIC` if it's failed. Many thanks for your great study and input.
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
tisonkun commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1288888042
@git-hulk I think that [`gettimeofday`](https://stackoverflow.com/questions/12392278/measure-time-in-linux-time-vs-clock-vs-getrusage-vs-clock-gettime-vs-gettimeof) is deprecated and we may instead replace `gettimeofday` with `clock_gettime` in the cpp code side?
--
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 closed issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
tisonkun closed issue #983: Flaky test on expire precision
URL: https://github.com/apache/incubator-kvrocks/issues/983
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
tisonkun commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1279893737
@git-hulk can we have an approach to guard the board of tests?
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
mapleFU commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289276065
> But our tests are all on the same machine. Will it be monotonic to use the same method to obtain the time on the same machine? If it is monotonic(go and c++ get time the same way), Then there will be no case of getting 11s in the failed test.
Golang's standard `Now` uses `CLOCK_MONOTONIC`, so, it's monotonic.
In C++, since it use `system_clock`, it uses `CLOCK_REALTIME`. `CLOCK_REALTIME` can be affected by NTP or setting, so it can move backward.
As a result:
1. Time api is different in golang and C++
2. The rounding would make problem worse
3. `CLOCK_REALTIME` may come accross time drifting
@tanruixiang
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
git-hulk commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1286431284
https://github.com/apache/incubator-kvrocks/actions/runs/3294519235/jobs/5432141189 also caused by the time precision.
--
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] tanruixiang commented on issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289258578
> > Our failing test case (Like `Extended SET EXAT option`) uses an expiration time in seconds, which doesn't seem to be a problem due to rounding. I think the main problem is the inconsistency in the way of getting time.
>
> At most of the time, the `value = expected_largest + 1`. Since clock may not have too large bias, it may be a sequence like:
>
> ```
> now: 10s
> now in kvrocks: 9.8s
> now + expire - kvrocks_time = 10s
> ```
>
> The 20ms' bias cames from different way of getting time, but it also could cames from NTP or others, since in C++, we just use system clock. @tanruixiang
But our tests are all on the same machine. Will it be monotonic to use the same method to obtain the time on the same machine? If it is monotonic(go and c++ get time the same way), Then there will be no case of getting 11s in the failed test.
```
go-redis get time:t1
set tll t1+10
get ttl from kvrocks : t2
if t1<=t2
=> t1+10 -t2 >=11
=> t1-t2>=1 is impossible
```
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
git-hulk commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1288812732
I have a basic idea to try hardening those test cases. From the current analysis, the only clue is the different behavior of getting current time in Go and C++, C++ will always use `gettimeofday` and Go is platform and runtime related(used `clock_gettime` in most Linux systems). So I think we can also use `gettimeofday` for those cases and have an eye on whether has this flaky condition or not.
Below code is from Go1.19's runtime:
`time_linux_amd64.s`
```
14 TEXT time·now<ABIInternal>(SB),NOSPLIT,$16-24
...
32 CMPQ R14, m_curg(BX) // Only switch if on curg.
33 JNE noswitch
38 noswitch:
39 SUBQ $32, SP // Space for two time results
40 ANDQ $~15, SP // Align for C code
41
42 MOVL $0, DI // CLOCK_REALTIME
43 LEAQ 16(SP), SI
44 MOVQ runtime·vdsoClockgettimeSym(SB), AX
45 CMPQ AX, $0
46 JEQ fallback
47 CALL AX
48
49 MOVL $1, DI // CLOCK_MONOTONIC
50 LEAQ 0(SP), SI
51 MOVQ runtime·vdsoClockgettimeSym(SB), AX
52 CALL AX
78 fallback:
79 MOVQ $SYS_clock_gettime, AX
80 SYSCALL
81
82 MOVL $1, DI // CLOCK_MONOTONIC
83 LEAQ 0(SP), SI
84 MOVQ $SYS_clock_gettime, AX
85 SYSCALL
86
87 JMP ret
```
`vdso_linux_amd64.go`
```
15 var vdsoSymbolKeys = []vdsoSymbolKey{
16 {"__vdso_gettimeofday", 0x315ca59, 0xb01bca00, &vdsoGettimeofdaySym},
17 {"__vdso_clock_gettime", 0xd35ec75, 0x6e43a318, &vdsoClockgettimeSym},
18 }
```
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
mapleFU commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1288892260
There are multiple `time` calling in kvrocks:
1. `std::time`, which is from c's api, setting the backup_time and checkpoint access time
2. `rocksdb::Env::GetCurrentTime`, which also calls `std::time` on posix
3. `gettimeofday` in `Worker::FeedMonitorConns`
Notes that:
1. `gettimeofday` is not monotonic, and can be updated by system: https://man7.org/linux/man-pages/man2/gettimeofday.2.html
2. std::time: https://en.cppreference.com/w/cpp/chrono/c/time
As for golang, after 1. Seems it uses monotonic clock ( https://pkg.go.dev/time#hdr-Monotonic_Clocks ). C++11 also provides monotonic clock: https://en.cppreference.com/w/cpp/chrono/steady_clock
In redis, I found an issue and the solving, maybe we could follow it: https://github.com/redis/redis/issues/416
At least, keep 3 or more `time` in kvrocks is really confusing.
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
git-hulk commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1279666122
@tisonkun After looking into those flaky test cases, I guess the reason is the way of getting timestamps. For example:
```
require.NoError(t, rdb.Do(ctx, "getex", "foo", "exat", time.Now().Add(10*time.Second).Unix()).Err())
util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5*time.Second, 10*time.Second)
```
The expiration timestamp is calculated in Go(wall clock) and TTL will use the C++ `gettimeofday` system.
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
mapleFU commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289234816
> Our failing test case (Like `Extended SET EXAT option`) uses an expiration time in seconds, which doesn't seem to be a problem due to rounding. I think the main problem is the inconsistency in the way of getting time.
At most of the time, the `value = expected_largest + 1`. Since clock may not have too large bias, it may be a sequence like:
```
now: 10s
now in kvrocks: 9.8s
now + expire - kvrocks_time = 10s
```
The 20ms' bias cames from different way of getting time, but it also could cames from NTP or others, since in C++, we just use system clock.
@tanruixiang
--
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] tanruixiang commented on issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1290110543
> @tisonkun @PragmaTwice @mapleFU @tanruixiang After rethinking about this issue, `gettimeofday`[1] is also using `clock_gettime`(as well as `chrono::system_clock`[2]) indeed and the only different is the time precision and Go `time.Now().Unix()` shouldn't be affected by the time precision(it's second unit), but we got 11s in test case which means the current second of Kvrocks was smaller than getting from Go. I guess the root cause is NTP synchronization backwards the unix second and nothing to do with using `gettimeofday` or `clock_gettime`, so #1038 can't fix current issue. Correct me if was wrong.
>
> [1] https://elixir.bootlin.com/glibc/latest/source/time/gettimeofday.c#L35 [2] https://codebrowser.dev/llvm/libcxx/src/chrono.cpp.html#_ZN6chronoL25__libcpp_system_clock_nowEv
Can we avoid this problem by disabling ntp, or configuring ntp? If we use the -x option, ntp will always use slew mode, which will not cause time back.
Reference:
https://groups.google.com/g/comp.protocols.time.ntp/c/u3bmMHV1g5o
https://docs.ntpsec.org/latest/ntpd.html
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
mapleFU commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289278855
Actually, I'm not saying that `rounding cause the problem`, I just declares that `it may made the problem worse`
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
tisonkun commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289204838
@git-hulk perhaps wait for one more occurrence ;-)
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
mapleFU commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289219893
> @mapleFU could you help with preparing a fix for [#983 (comment)](https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289131135)?
@tisonkun Glad to fix it, but should I waiting for #1032 to be merged first?
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
git-hulk commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1290041997
@tisonkun @PragmaTwice @mapleFU @tanruixiang After rethinking about this issue, `gettimeofday`[1] is also using `clock_gettime`(as well as `chrono::system_clock`[2]) indeed and the only different is the time precision and Go `time.Now().Unix()` shouldn't be affected by the time precision(it's second unit), but we got 11s in test case which means the current second of Kvrocks was smaller than getting from Go. I guess the root cause is NTP synchronization backwards the unix second and nothing to do with `gettimeofday` and `clock_gettime`, so #1038 can't fix current issue. Correct me if was wrong.
[1] https://elixir.bootlin.com/glibc/latest/source/time/gettimeofday.c#L35
[2] https://codebrowser.dev/llvm/libcxx/src/chrono.cpp.html#_ZN6chronoL25__libcpp_system_clock_nowEv
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
tisonkun commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289912199
> @tanruixiang @git-hulk Maybe I made a mistake, after go through go's `time.Now`, the `nsecs` just return a wall time if possible. Let's just waiting for testing now
OK. Closed as completed. Reopen or relax the upper bound if it happens again.
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
tisonkun commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1277039199
It's interesting that the TTL even "longer" than what is set.
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
mapleFU commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289131135
Holdon, I think here is another problem in `ParseTTL`, which would cause this problem, after go through the code of `ParseTTL` in `redis_cmd.cc`, I found that:
1. `expire` is rounded to `seconds`
2. `now` is also rounded to `seconds`
And assume that:
```
now: 10.99s
expire: 21.01s
expected: 10.02s -> 10s ( bias for 20ms between clock maybe ok )
currently: 11s
```
@git-hulk
--
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 closed issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
PragmaTwice closed issue #983: Flaky test on expire precision
URL: https://github.com/apache/incubator-kvrocks/issues/983
--
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] tanruixiang commented on issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289288324
> Actually, I'm not saying that `rounding cause the problem`, I just declares that `it may made the problem worse`. Since the two clocks maybe just have ~10ms' bias. And it maybe not harmful, because one second may only harm our testings ╮(￣▽￣"")╭
It seems that the main problem is still go and c++ using different clocks. Rounding in the same direction won't cause a problem if use the same clocks.
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
tisonkun commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289912298
> @tanruixiang @git-hulk Maybe I made a mistake, after go through go's `time.Now`, the `nsecs` just return a wall time if possible. Let's just waiting for testing now
OK. Closed as completed. Reopen or relax the upper bound if it happens again.
--
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] tanruixiang commented on issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289220603
> Holdon, I think here is another problem in `ParseTTL`, which would cause this problem, after go through the code of `ParseTTL` in `redis_cmd.cc`, I found that:
>
> 1. `expire` is rounded to `seconds`
> 2. `now` is also rounded to `seconds`
>
> And assume that:
>
> ```
> now: 10.99s
> expire: 21.01s
> expected: 10.02s -> 10s ( bias for 20ms between clock maybe ok )
> currently: 11s
> ```
>
> @git-hulk
Our failing test case (Like `Extended SET EXAT option`) uses an expiration time in seconds, which doesn't seem to be a problem due to rounding.
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
git-hulk commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1279927912
@git-hulk Yes, will be harden by increasing the upper bound for all tests which used EXAT instrument.
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
git-hulk commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1288931255
> @git-hulk I think that [gettimeofday](https://stackoverflow.com/questions/12392278/measure-time-in-linux-time-vs-clock-vs-getrusage-vs-clock-gettime-vs-gettimeof) is deprecated and we may instead replace gettimeofday with clock_gettime in the cpp code side?
Thanks, I didn't notice that `gettimeofday` is deprecated, that's a sure thing since we should NOT use the deprecated API if possible.
> At least, keep 3 or more time in kvrocks is really confusing.
@mapleFU Thanks for your nice input. You're right we should use a coherent way to get time.
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
mapleFU commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289149266
> 🤣
OK, seems that we should using millseconds to `minus` and do rounding 🤣. So trickey for fixing tests
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
git-hulk commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289208770
okay
--
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 issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
tisonkun commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289214326
@mapleFU could you help with preparing a fix for https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289131135?
--
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] tanruixiang commented on issue #983: Flaky test on expire precision
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on issue #983:
URL: https://github.com/apache/incubator-kvrocks/issues/983#issuecomment-1289284358
> > But our tests are all on the same machine. Will it be monotonic to use the same method to obtain the time on the same machine? If it is monotonic(go and c++ get time the same way), Then there will be no case of getting 11s in the failed test.
>
> Golang's standard `Now` uses `CLOCK_MONOTONIC`, so, it's monotonic.
>
> In C++, since it use `system_clock`, it uses `CLOCK_REALTIME`. `CLOCK_REALTIME` can be affected by NTP or setting, so it can move backward.
>
> As a result:
>
> 1. Time api is different in golang and C++
> 2. The rounding would make problem worse
> 3. `CLOCK_REALTIME` may come accross time drifting
>
> @tanruixiang
Ok. I misunderstood that after this [modification](https://github.com/apache/incubator-kvrocks/pull/1038), Go and C++ would use the same clock. Thank you for your patience.
--
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