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

server/etcdserver/api/etcdhttp: exclude the same alarm type activated by multiple peers #13467

Merged
merged 2 commits into from
Nov 14, 2021

Conversation

chaochn47
Copy link
Member

@chaochn47 chaochn47 commented Nov 9, 2021

#12880 failed to exclude all alarms activated by multiple members for a given alarm type.

Reproduce

etcd version 3.4.17

$ rm -rf infra1.etcd infra2.etcd infra3.etcd ~/log/etcd/infra*
$ ./bin/etcd --name infra1 \
--listen-client-urls http://127.0.0.1:2379 \
--advertise-client-urls http://127.0.0.1:2379 \
--listen-peer-urls http://127.0.0.1:12380 \
--initial-advertise-peer-urls http://127.0.0.1:12380 \
--initial-cluster-token etcd-cluster-1 --initial-cluster 'infra1=http://127.0.0.1:12380,infra2=http://127.0.0.1:22380,infra3=http://127.0.0.1:32380' --initial-cluster-state new \
--quota-backend-bytes 200000 > ~/log/etcd/infra1.log 2>&1 &

$ ./bin/etcd --name infra2 \
--listen-client-urls http://127.0.0.1:22379 \
--advertise-client-urls http://127.0.0.1:22379 \
--listen-peer-urls http://127.0.0.1:22380 \
--initial-advertise-peer-urls http://127.0.0.1:22380 \
--initial-cluster-token etcd-cluster-1 --initial-cluster 'infra1=http://127.0.0.1:12380,infra2=http://127.0.0.1:22380,infra3=http://127.0.0.1:32380' --initial-cluster-state new \
--quota-backend-bytes 200000 > ~/log/etcd/infra2.log 2>&1 &

$ ./bin/etcd --name infra3 \
--listen-client-urls http://127.0.0.1:32379 \
--advertise-client-urls http://127.0.0.1:32379 \
--listen-peer-urls http://127.0.0.1:32380 \
--initial-advertise-peer-urls http://127.0.0.1:32380 \
--initial-cluster-token etcd-cluster-1 --initial-cluster 'infra1=http://127.0.0.1:12380,infra2=http://127.0.0.1:22380,infra3=http://127.0.0.1:32380' --initial-cluster-state new \
--quota-backend-bytes 200000 > ~/log/etcd/infra3.log 2>&1 &

$ for n in {1..3868}; do sleep 0.1; dd if=/dev/urandom bs=1024 count=17 | ETCDCTL_API=3 etcdctl --endpoints http://localhost:2379,http://127.0.0.1:22379,http://127.0.0.1:32379  put /registry/pods/${n} ; done
$ ctrl+c

$ etcdctl alarm list
memberID:9372538179322589801 alarm:NOSPACE
memberID:10501334649042878790 alarm:NOSPACE
memberID:18249187646912138824 alarm:NOSPACE

$ curl "http://localhost:2379/health?exclude=NOSPACE"
{"health":"false"}

# etcd log
2021-11-09 06:01:51.367165 W | etcdserver/api/etcdhttp: /health error due to memberID:10501334649042878790 alarm:NOSPACE

After this PR

curl "http://localhost:2379/health?exclude=NOSPACE"
{"health":"true","reason":""}

PTAL @hexfusion @gyuho

Copy link
Contributor

@hexfusion hexfusion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@hexfusion
Copy link
Contributor

please update changelogs for backports thanks!

Copy link
Member

@spzala spzala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm
Thanks!

@chaochn47
Copy link
Member Author

Updated the change log with the backport PRs. PTAL thanks!!

@hexfusion hexfusion merged commit 1577cdd into etcd-io:main Nov 14, 2021
hexfusion added a commit that referenced this pull request Nov 14, 2021
cherry-pick to 3.5 from #13467 exclude the same alarm type activated by multiple peers
hexfusion added a commit that referenced this pull request Nov 14, 2021
backport 3.4 from #13467 exclude the same alarm type activated by multiple peers
@chaochn47 chaochn47 deleted the fix_exclude_alarms branch November 15, 2021 18:30
hasbro17 pushed a commit to hasbro17/etcd that referenced this pull request Feb 2, 2022
cherry-pick to 3.5 from etcd-io#13467 exclude the same alarm type activated by multiple peers
tjungblu pushed a commit to tjungblu/etcd that referenced this pull request Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants