Skip to content

[3.4] etcdserver: add cluster id check for hashKVHandler#16995

Merged
ahrtr merged 1 commit intoetcd-io:release-3.4from
ahrtr:3.4_clusterId_20231122
Nov 22, 2023
Merged

[3.4] etcdserver: add cluster id check for hashKVHandler#16995
ahrtr merged 1 commit intoetcd-io:release-3.4from
ahrtr:3.4_clusterId_20231122

Conversation

@ahrtr
Copy link
Copy Markdown
Member

@ahrtr ahrtr commented Nov 22, 2023

@ahrtr ahrtr mentioned this pull request Nov 22, 2023
Closed
14 tasks
@ahrtr ahrtr force-pushed the 3.4_clusterId_20231122 branch from 377a57a to 707e628 Compare November 22, 2023 16:50
serathius
serathius reviewed Nov 22, 2023
View reviewed changes
Comment thread etcdserver/corrupt_test.go Outdated
serathius
serathius reviewed Nov 22, 2023
View reviewed changes
Comment thread etcdserver/corrupt_test.go Outdated
@ahrtr ahrtr force-pushed the 3.4_clusterId_20231122 branch from 707e628 to 07869cb Compare November 22, 2023 16:55
@ahrtr
etcdserver: add cluster id check for hashKVHandler
c750e01 
backport etcd-io#15924 to 3.4

Signed-off-by: Benjamin Wang <benjamin.ahrtr@gmail.com>
@ahrtr ahrtr force-pushed the 3.4_clusterId_20231122 branch from 07869cb to c750e01 Compare November 22, 2023 16:55
@serathius
Copy link
Copy Markdown
Member

serathius commented Nov 22, 2023

For posterity it's worth to mention this backport also includes improvements to TestInPlaceRecovery from #16677.

@ahrtr ahrtr merged commit 6e6b615 into etcd-io:release-3.4 Nov 22, 2023
fuweid added a commit to fuweid/etcd that referenced this pull request Jan 23, 2024
@fuweid
fix: linux-amd64-e2e job timeout issue
3c4f9ff 
Assume etcd-io#16916 as baseline. The E2E takes `1395.082s`.

* etcd-io#16988

It introduced `TestAuthority` which takes `18.39s`.
And after etcd-io#16997, it takes `50.05s`.

* etcd-io#16995

It introduced `TestInPlaceRecovery` which takes `17.37s`.

* etcd-io#17144

  - New `TestHTTPHealthHandler` takes `29.9s`
  - New `TestHTTPLivezReadyzHandler` takes `35.20s`

* etcd-io#17173

  - New `TestMemberReplace` takes `7.55s`.

Ideally, it should increase `140.07s`. It's not larger than `1800s`
timeout value.

However, we run E2E cases 3 times. By default, we run E2E cases with
`-cpu 1,2,4`. That means that we run 3 times.

```bash
$ go help testflag

 -count n
            Run each test, benchmark, and fuzz seed n times (default 1).

            If -cpu is set, run n times for each GOMAXPROCS value.
            Examples are always run once. -count does not apply to
            fuzz tests matched by -fuzz.
```

I don't think we should run E2E with different GOMAXPROCS value. All the
`TestXYZ` are used to control etcd process and we don't set GOMAXPROCS
env to etcd process. So, we don't need `-cpu` setting for E2E.

Closes: etcd-io#17241

Signed-off-by: Wei Fu <fuweid89@gmail.com>
@fuweid fuweid mentioned this pull request Jan 23, 2024
Merged
fuweid added a commit to fuweid/etcd that referenced this pull request Jan 23, 2024
@fuweid
fix: linux-amd64-e2e job timeout issue
1d47674 
Assume etcd-io#16916 as baseline. The E2E takes `1395.082s`.

* etcd-io#16988

It introduced `TestAuthority` which takes `18.39s`.
And after etcd-io#16997, it takes `50.05s`.

* etcd-io#16995

It introduced `TestInPlaceRecovery` which takes `17.37s`.

* etcd-io#17144

  - New `TestHTTPHealthHandler` takes `29.9s`
  - New `TestHTTPLivezReadyzHandler` takes `35.20s`

* etcd-io#17173

  - New `TestMemberReplace` takes `7.55s`.

Ideally, it should increase `140.07s`. It's not larger than `1800s`
timeout value.

However, we run E2E cases 3 times. By default, we run E2E cases with
`-cpu 1,2,4`. That means that we run 3 times.

`1395.082s` + `140.07s * 3` = `1815.292s` > `1800s`

```bash
$ go help testflag

 -count n
            Run each test, benchmark, and fuzz seed n times (default 1).

            If -cpu is set, run n times for each GOMAXPROCS value.
            Examples are always run once. -count does not apply to
            fuzz tests matched by -fuzz.
```

I don't think we should run E2E with different GOMAXPROCS value. All the
`TestXYZ` are used to control etcd process and we don't set GOMAXPROCS
env to etcd process. So, we don't need `-cpu` setting for E2E.

Closes: etcd-io#17241

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this pull request Jan 23, 2024
@fuweid
fix: linux-amd64-e2e job timeout issue
a511d63 
Assume etcd-io#16916 as baseline. The E2E takes `1395.082s`.

* etcd-io#16988

It introduced `TestAuthority` which takes `18.39s`.
And after etcd-io#16997, it takes `50.05s`.

* etcd-io#16995

It introduced `TestInPlaceRecovery` which takes `17.37s`.

* etcd-io#17144

  - New `TestHTTPHealthHandler` takes `29.9s`
  - New `TestHTTPLivezReadyzHandler` takes `35.20s`

* etcd-io#17173

  - New `TestMemberReplace` takes `7.55s`.

Ideally, it should increase `140.07s`. It's not larger than `1800s`
timeout value.

However, we run E2E cases 3 times. By default, we run E2E cases with
`-cpu 1,2,4`. That means that we run 3 times.

`1395.082s` + `140.07s * 3` = `1815.292s` > `1800s`

```bash
$ go help testflag

 -count n
            Run each test, benchmark, and fuzz seed n times (default 1).

            If -cpu is set, run n times for each GOMAXPROCS value.
            Examples are always run once. -count does not apply to
            fuzz tests matched by -fuzz.
```

I don't think we should run E2E with different GOMAXPROCS value. All the
`TestXYZ` are used to control etcd process and we don't set GOMAXPROCS
env to etcd process.

Set `CPU=4` to align with main and release/3.5.

Closes: etcd-io#17241

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this pull request Jan 23, 2024
@fuweid
fix: linux-amd64-e2e job timeout issue
c74cc42 
Assume etcd-io#16916 as baseline. The E2E takes `1395.082s`.

* etcd-io#16988

It introduced `TestAuthority` which takes `18.39s`.
And after etcd-io#16997, it takes `50.05s`.

* etcd-io#16995

It introduced `TestInPlaceRecovery` which takes `17.37s`.

* etcd-io#17144

  - New `TestHTTPHealthHandler` takes `29.9s`
  - New `TestHTTPLivezReadyzHandler` takes `35.20s`

* etcd-io#17173

  - New `TestMemberReplace` takes `7.55s`.

Ideally, it should increase `140.07s`. It's not larger than `1800s`
timeout value.

However, we run E2E cases 3 times. By default, we run E2E cases with
`-cpu 1,2,4`. That means that we run 3 times.

`1395.082s` + `140.07s * 3` = `1815.292s` > `1800s`

```bash
$ go help testflag

 -count n
            Run each test, benchmark, and fuzz seed n times (default 1).

            If -cpu is set, run n times for each GOMAXPROCS value.
            Examples are always run once. -count does not apply to
            fuzz tests matched by -fuzz.
```

I don't think we should run E2E with different GOMAXPROCS value. All the
`TestXYZ` are used to control etcd process and we don't set GOMAXPROCS
env to etcd process.

Set `CPU=4` to align with main and release/3.5.

Closes: etcd-io#17241

Signed-off-by: Wei Fu <fuweid89@gmail.com>
@ahrtr ahrtr deleted the 3.4_clusterId_20231122 branch June 12, 2024 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serathius serathius serathius approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ahrtr @serathius