Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

After setting the time zone (not the default time zone), there is an issue with displaying daylight saving time #49586

Closed
knull-cn opened this issue Dec 19, 2023 · 7 comments · Fixed by tikv/tikv#16221
Assignees
Labels
affects-4.0 This bug affects 4.0.x versions. affects-5.0 This bug affects 5.0.x versions. affects-5.1 This bug affects 5.1.x versions. affects-5.2 This bug affects 5.2.x versions. affects-5.3 This bug affects 5.3.x versions. affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.0 affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.2 affects-6.3 affects-6.4 affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-6.6 affects-7.0 affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.2 affects-7.3 affects-7.4 affects-7.5 This bug affects the 7.5.x(LTS) versions. severity/critical sig/execution SIG execution sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.

Comments

@knull-cn
Copy link
Contributor

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

SELECT @@global.time_zone, @@session.time_zone, @@global.system_time_zone;
set time_zone = 'Brazil/East';

create table jt(
`id` bigint(20) NOT NULL COMMENT '主键ID',
`waybill_time` timestamp NULL DEFAULT NULL COMMENT '',
`sign_time` timestamp NULL DEFAULT NULL COMMENT '',
`bill_generation_time` timestamp DEFAULT CURRENT_TIMESTAMP COMMENT '',
`settle_financial_center_id` int(11) DEFAULT NULL COMMENT '',
`describe_id` int(11) DEFAULT NULL COMMENT '',
`merge_waybill` tinyint(4) DEFAULT 0 COMMENT '',
PRIMARY KEY (`id`) /*T![clustered_index] CLUSTERED */,
KEY `idx_new_stime_sdm` (`sign_time`,`settle_financial_center_id`,`describe_id`,`merge_waybill`)
);

INSERT INTO `jt`(`id`, `waybill_time`, `sign_time`, `bill_generation_time`) VALUES (598212559966838838, '2023-11-30 14:02:00', '2023-11-30 17:22:11', '2023-11-30 14:02:08');

-- 这两个query时间显示不一样
select * from jt where id = 598212559966838838;  -- 这里是对的
select * from jt ;  -- 这里对了一小时

2. What did you expect to see? (Required)

show right time

3. What did you see instead (Required)

show wrong time( The time display is 1 hour longer)
image

4. What is your TiDB version? (Required)

Problems were found during testing in the following versions: v6.5.3/6.5.6/7.1.2/7.5.0.
I estimate that all versions have issues

@knull-cn knull-cn added the type/bug The issue is confirmed as a bug. label Dec 19, 2023
@jebter jebter added sig/execution SIG execution sig/sql-infra SIG: SQL Infra severity/critical affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. labels Dec 20, 2023
@ti-chi-bot ti-chi-bot bot added may-affects-5.4 This bug maybe affects 5.4.x versions. may-affects-6.1 labels Dec 20, 2023
@tiancaiamao tiancaiamao self-assigned this Dec 21, 2023
@tiancaiamao
Copy link
Contributor

We need to fix it on the tikv side.

Observations:

  1. point get has no problem with this, because point get protocal get the raw key-value data as []byte and handle the decoding on tidb side, timezone is handled here:
goroutine 1748 [running]:
runtime/debug.Stack()
        /home/genius/project/go/src/runtime/debug/stack.go:24 +0x5e
runtime/debug.PrintStack()
        /home/genius/project/go/src/runtime/debug/stack.go:16 +0x13
github.com/pingcap/tidb/pkg/types.(*Time).ConvertTimeZone(0xc01438a548, 0x0?, 0xc00609aa80)
        /home/genius/project/src/github.com/pingcap/tidb/pkg/types/time.go:372 +0xe8
github.com/pingcap/tidb/pkg/util/rowcodec.(*ChunkDecoder).decodeColToChunk(0xc05bd304e0, 0xc05a6e5dc0?, 0xc05b0273c8, {0xc05a6e5ded?, 0xc0598bf1e8?, 0xc01438a670?}, 0x2d378d1?)
        /home/genius/project/src/github.com/pingcap/tidb/pkg/util/rowcodec/decoder.go:328 +0x749
github.com/pingcap/tidb/pkg/util/rowcodec.(*ChunkDecoder).DecodeToChunk(0xc05bd304e0, {0xc05a6e5dc0?, 0x63850d0?, 0xc02bd629f0?}, {0x63abc60, 0x90a3208}, 0x13?)        /home/genius/project/src/github.com/pingcap/tidb/pkg/util/rowcodec/decoder.go:227 +0x285
github.com/pingcap/tidb/pkg/executor.DecodeRowValToChunk({0x63fb978?, 0xc00e826280?}, 0x13?, 0x13?, {0x63abc60?, 0x90a3208?}, {0xc05a6e5dc0?, 0x934c680?, 0xc01438a7c8?}, 0xc077ca5950, ...)
        /home/genius/project/src/github.com/pingcap/tidb/pkg/executor/point_get.go:588 +0x53
github.com/pingcap/tidb/pkg/executor.(*PointGetExecutor).Next(0xc05ad7f1e0, {0x63850d0, 0xc02bd629f0}, 0xc077ca5950)
        /home/genius/project/src/github.com/pingcap/tidb/pkg/executor/point_get.go:336 +0x95f
github.com/pingcap/tidb/pkg/executor/internal/exec.Next({0x63850d0, 0xc02bd629f0}, {0x63acc60, 0xc05ad7f1e0}, 0xc077ca5950)
        /home/genius/project/src/github.com/pingcap/tidb/pkg/executor/internal/exec/executor.go:309 +0x26c
github.com/pingcap/tidb/pkg/executor.(*ExecStmt).next(0xc05b9c4960, {0x63850d0, 0xc02bd629f0}, {0x63acc60, 0xc05ad7f1e0}, 0x2d0a2b3?)
        /home/genius/project/src/github.com/pingcap/tidb/pkg/executor/adapter.go:1245 +0x6e
github.com/pingcap/tidb/pkg/executor.(*recordSet).Next(0xc05ba95f80, {0x63850d0?, 0xc02bd629f0?}, 0xc077ca5950)
        /home/genius/project/src/github.com/pingcap/tidb/pkg/executor/adapter.go:156 +0xb2
github.com/pingcap/tidb/pkg/server/internal/resultset.(*tidbResultSet).Next(0x59c3b25?, {0x63850d0?, 0xc02bd629f0?}, 0x1bd8928?)
        /home/genius/project/src/github.com/pingcap/tidb/pkg/server/internal/resultset/resultset.go:64 +0x25
github.com/pingcap/tidb/pkg/server.(*clientConn).writeChunks(0xc07329f040, {0x63850d0, 0xc02bd629f0}, {0x639cc18, 0xc05bb82800}, 0x0, 0x1bc?)
        /home/genius/project/src/github.com/pingcap/tidb/pkg/server/conn.go:2269 +0x18a
github.com/pingcap/tidb/pkg/server.(*clientConn).writeResultSet(0xc07329f040, {0x63850d0, 0xc02bd629f0}, {0x639cc18, 0xc05bb82800}, 0x98?, 0x2, 0x0?)
        /home/genius/project/src/github.com/pingcap/tidb/pkg/server/conn.go:2212 +0x2f0
github.com/pingcap/tidb/pkg/server.(*clientConn).handleStmt(0xc07329f040, {0x6385108, 0xc05b5dbc70}, {0x6399f08?, 0xc05bd40480}, {0x0, 0x0, 0x0}, 0x1)
        /home/genius/project/src/github.com/pingcap/tidb/pkg/server/conn.go:2005 +0x52a
github.com/pingcap/tidb/pkg/server.(*clientConn).handleQuery(0xc07329f040, {0x6385108, 0xc05b5dbc70}, {0xc059ee5041, 0x41})
        /home/genius/project/src/github.com/pingcap/tidb/pkg/server/conn.go:1748 +0x9a5
github.com/pingcap/tidb/pkg/server.(*clientConn).dispatch(0xc07329f040, {0x63850d0?, 0xc069559bc0?}, {0xc059ee5040, 0x42, 0x42})
        /home/genius/project/src/github.com/pingcap/tidb/pkg/server/conn.go:1322 +0xf8b
github.com/pingcap/tidb/pkg/server.(*clientConn).Run(0xc07329f040, {0x63850d0, 0xc069559bc0})
        /home/genius/project/src/github.com/pingcap/tidb/pkg/server/conn.go:1101 +0x53e
github.com/pingcap/tidb/pkg/server.(*Server).onConn(0xc00ecc6800?, 0xc07329f040)        /home/genius/project/src/github.com/pingcap/tidb/pkg/server/server.go:701 +0x89d
created by github.com/pingcap/tidb/pkg/server.(*Server).startNetworkListener in goroutine 1341
        /home/genius/project/src/github.com/pingcap/tidb/pkg/server/server.go:517 +0x77f
  1. using unistore, the result is also correct. In this case, it's using chunk protocol and the unistore's coprocessor layer handle the timezone,
goroutine 13603 [running]:
runtime/debug.Stack()
        /home/genius/project/go/src/runtime/debug/stack.go:24 +0x5e
runtime/debug.PrintStack()
        /home/genius/project/go/src/runtime/debug/stack.go:16 +0x13
github.com/pingcap/tidb/pkg/types.(*Time).ConvertTimeZone(0xc000eedc08, 0x0?, 0xc0329c2a10)
        /home/genius/project/src/github.com/pingcap/tidb/pkg/types/time.go:372 +0xe8
github.com/pingcap/tidb/pkg/util/rowcodec.(*ChunkDecoder).decodeColToChunk(0xc0329b09c0, 0x7fd208156449?, 0xc0329c6f68, {0x7fd20815646b?, 0xc0329ac6b8?, 0xc0329b5040?}, 0x3fff8ac?)
        /home/genius/project/src/github.com/pingcap/tidb/pkg/util/rowcodec/decoder.go:328 +0x749
github.com/pingcap/tidb/pkg/util/rowcodec.(*ChunkDecoder).DecodeToChunk(0xc0329b09c0, {0x7fd208156449?, 0x40?, 0x12?}, {0x63abc60, 0xc0329ac6b8}, 0x3fde8bc?)
        /home/genius/project/src/github.com/pingcap/tidb/pkg/util/rowcodec/decoder.go:227 +0x285
github.com/pingcap/tidb/pkg/store/mockstore/unistore/cophandler.(*tableScanExec).Process(0xc032676780, {0xc032051180, 0x13, 0x40}, {0x7fd208156449, 0x2b, 0x265d43})
        /home/genius/project/src/github.com/pingcap/tidb/pkg/store/mockstore/unistore/cophandler/mpp_exec.go:144 +0xaa
github.com/pingcap/tidb/pkg/store/mockstore/unistore/tikv/dbreader.(*DBReader).Scan(0xc0329ae840, {0xc031d05c98, 0x13, 0x13}, {0xc031d05cb0, 0x14, 0x14}, 0x7fffffffffffffff, 0x6322cec00a00000, {0x636d9b8, ...})
        /home/genius/project/src/github.com/pingcap/tidb/pkg/store/mockstore/unistore/tikv/dbreader/db_reader.go:240 +0x3b2
github.com/pingcap/tidb/pkg/store/mockstore/unistore/cophandler.(*tableScanExec).open.func1()
        /home/genius/project/src/github.com/pingcap/tidb/pkg/store/mockstore/unistore/cophandler/mpp_exec.go:194 +0x218
github.com/pingcap/tidb/pkg/util.(*WaitGroupWrapper).Run.func1()
        /home/genius/project/src/github.com/pingcap/tidb/pkg/util/wait_group_wrapper.go:157 +0x4f
created by github.com/pingcap/tidb/pkg/util.(*WaitGroupWrapper).Run in goroutine 13489
        /home/genius/project/src/github.com/pingcap/tidb/pkg/util/wait_group_wrapper.go:155 +0x73
  1. If work with tikv, the bug reproduce.

There are two possible reason, one is that tikv coprocessor does not handle the ConvertTimeZone correctly.
The other reason is the tikv does handle it, but the time zone library between Go and Rust differs, so when handling daylight saving timezone, the result differs.

@tiancaiamao
Copy link
Contributor

use set @@tidb_enable_chunk_rpc = 0 can workaround this bug, because without using the chunk protocol,
tidb read the raw key-value data from tikv and decode in tidb layer. handling convert time zone correctly.

@breezewish
Copy link
Member

breezewish commented Dec 21, 2023

After investigation, the problem is TiKV is handling this date time incorrectly.

The code below reproduces how TiKV deal with this date time:

use chrono::{TimeZone, NaiveDate, DateTime};
fn chrono_datetime<T: TimeZone>(
    time_zone: &T,
    year: u32,
    month: u32,
    day: u32,
    hour: u32,
    minute: u32,
    second: u32,
    micro: u32,
) -> DateTime<T> {
    // NOTE: We are not using `tz::from_ymd_opt` as suggested in chrono's README due
    // to chronotope/chrono-tz #23.
    // As a workaround, we first build a NaiveDate, then attach time zone
    // information to it.
    NaiveDate::from_ymd_opt(year as i32, month, day)
        .and_then(|date| date.and_hms_opt(hour, minute, second))
        .and_then(|t| t.checked_add_signed(chrono::Duration::microseconds(i64::from(micro))))
        .and_then(|datetime| time_zone.from_local_datetime(&datetime).earliest())
        .unwrap()
}
fn main() {
    use std::str::FromStr;
    let utc = chrono_datetime(&chrono_tz::UTC, 2023, 11, 30, 17, 2, 0, 0);
    let tz = chrono_tz::Tz::from_str("Brazil/East").ok().unwrap();
    let timestamp = tz.from_utc_datetime(&utc.naive_utc());
    let naive_local = timestamp.naive_local();
    println!("time={:?}", naive_local);
}

When using the current TiKV's dependency:

[dependencies]
chrono = "0.4.11"
chrono-tz = "=0.5.1"

We will get the wrong result you observed:

time=2023-11-30T15:02:00

Use a newer dependency version could fix it:

[dependencies]
chrono = "0.4.11"
chrono-tz = "=0.5.3"

We will get a correct result:

time=2023-11-30T14:02:00

@breezewish
Copy link
Member

breezewish commented Dec 21, 2023

DST is no longer used in Brazil since 2020 [ref]. Updating chrono-tz from 0.5.1 to 0.5.2 bumps the timezone database from 2018i to 2020a, which includes this change, thus fixes the issue.

Update: There are several news indicating that Brazil is considering bring back DST [ref(Chinese)] [ref(Chinese)] so that we need to keep track on the time zone updates in future.

See TZ changelog: https://data.iana.org/time-zones/tzdb/NEWS

@overvenus overvenus added affects-4.0 This bug affects 4.0.x versions. affects-5.0 This bug affects 5.0.x versions. affects-5.1 This bug affects 5.1.x versions. affects-5.2 This bug affects 5.2.x versions. affects-5.3 This bug affects 5.3.x versions. affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.0 affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.2 affects-6.3 affects-6.4 affects-6.6 and removed may-affects-5.4 This bug maybe affects 5.4.x versions. may-affects-6.1 labels Dec 22, 2023
ti-chi-bot bot pushed a commit to tikv/tikv that referenced this issue Dec 22, 2023
…16221)

ref #16220, close pingcap/tidb#49586

Brazil no longer observes DST since 2020[1]. Updating chrono-tz from
0.5.1 to 0.5.2 bumps the timezone database from 2018i to 2020a, which
includes this change, thus fixes the issue.

[1]: https://en.wikipedia.org/wiki/Daylight_saving_time_in_Brazil

Signed-off-by: Neil Shen <[email protected]>

Co-authored-by: Wenxuan <[email protected]>
@Defined2014 Defined2014 reopened this Dec 22, 2023
ti-chi-bot bot pushed a commit to tikv/tikv that referenced this issue Dec 25, 2023
…16221) (#16225)

ref #16220, close pingcap/tidb#49586

Brazil no longer observes DST since 2020[1]. Updating chrono-tz from
0.5.1 to 0.5.2 bumps the timezone database from 2018i to 2020a, which
includes this change, thus fixes the issue.

[1]: https://en.wikipedia.org/wiki/Daylight_saving_time_in_Brazil

Signed-off-by: Neil Shen <[email protected]>

Co-authored-by: Neil Shen <[email protected]>
Co-authored-by: Wenxuan <[email protected]>
@tiancaiamao
Copy link
Contributor

In fact, #29427 is the same issue with this one.
But we'd better keep that one open until we reach a consensus about how to maintain the tzdata on both tidb and tikv side in the future.

@breezewish
Copy link
Member

I agree that we should keep this issue open, as currently there are only workarounds.

@XuHuaiyu
Copy link
Contributor

An individual issue would be better. Keeping this bug issue in an open state consistently will impact subsequent release actions. 😄
Keep tracking in #49848

ti-chi-bot bot added a commit to tikv/tikv that referenced this issue Jan 31, 2024
…16221) (#16227)

ref #16220, ref pingcap/tidb#49586

Brazil no longer observes DST since 2020[1]. Updating chrono-tz from
0.5.1 to 0.5.2 bumps the timezone database from 2018i to 2020a, which
includes this change, thus fixes the issue.

[1]: https://en.wikipedia.org/wiki/Daylight_saving_time_in_Brazil

Signed-off-by: Neil Shen <[email protected]>

Co-authored-by: Neil Shen <[email protected]>
Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
Co-authored-by: Wenxuan <[email protected]>
ti-chi-bot bot added a commit to tikv/tikv that referenced this issue Feb 27, 2024
…16221) (#16226)

ref #16220, ref pingcap/tidb#49586

Brazil no longer observes DST since 2020[1]. Updating chrono-tz from
0.5.1 to 0.5.2 bumps the timezone database from 2018i to 2020a, which
includes this change, thus fixes the issue.

[1]: https://en.wikipedia.org/wiki/Daylight_saving_time_in_Brazil

Signed-off-by: Neil Shen <[email protected]>

Co-authored-by: Neil Shen <[email protected]>
Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
Co-authored-by: Wenxuan <[email protected]>
ti-chi-bot bot pushed a commit to tikv/tikv that referenced this issue Feb 27, 2024
…16221) (#16224)

ref #16220, ref pingcap/tidb#49586

Brazil no longer observes DST since 2020[1]. Updating chrono-tz from
0.5.1 to 0.5.2 bumps the timezone database from 2018i to 2020a, which
includes this change, thus fixes the issue.

[1]: https://en.wikipedia.org/wiki/Daylight_saving_time_in_Brazil

Signed-off-by: Neil Shen <[email protected]>

Co-authored-by: Neil Shen <[email protected]>
Co-authored-by: Wenxuan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-4.0 This bug affects 4.0.x versions. affects-5.0 This bug affects 5.0.x versions. affects-5.1 This bug affects 5.1.x versions. affects-5.2 This bug affects 5.2.x versions. affects-5.3 This bug affects 5.3.x versions. affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.0 affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.2 affects-6.3 affects-6.4 affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-6.6 affects-7.0 affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.2 affects-7.3 affects-7.4 affects-7.5 This bug affects the 7.5.x(LTS) versions. severity/critical sig/execution SIG execution sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants