Skip to content

ETCD-352: Adding etcd defrag goroutine#1395

Merged
openshift-merge-robot merged 1 commit intoopenshift:mainfrom
dusk125:etcd-defrag
Mar 1, 2023
Merged

ETCD-352: Adding etcd defrag goroutine#1395
openshift-merge-robot merged 1 commit intoopenshift:mainfrom
dusk125:etcd-defrag

Conversation

@dusk125
Copy link
Contributor

@dusk125 dusk125 commented Feb 21, 2023

This is the implementation for the 'defrag goroutine' approach from this enhancement

@openshift-ci-robot
Copy link

openshift-ci-robot commented Feb 21, 2023 •
edited by openshift-ci bot
Loading

@dusk125: This pull request references ETCD-352 which is a valid jira issue.

Details

In response to this:

This is the implementation for the 'defrag goroutine' approach from this enhancement

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Feb 21, 2023
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 21, 2023
@openshift-ci openshift-ci bot requested review from copejon and oglok February 21, 2023 16:16
dusk125
dusk125 commented Feb 21, 2023
View reviewed changes
etcd/cmd/microshift-etcd/run.go Outdated
Copy link
Contributor Author

@dusk125 dusk125 Feb 21, 2023

Choose a reason for hiding this comment

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

In Openshift, the operator will retry a failing defrag 3 times before marking the etcd node degraded.
We could retry (configurable a default of) 3 times before throwing an error and stopping things, or maybe even allow the user to specify a value that just turns it off? Thoughts?

@dusk125 dusk125 force-pushed the etcd-defrag branch 3 times, most recently from 516f251 to 98f6b8e Compare February 22, 2023 11:05
pmtk
pmtk reviewed Feb 22, 2023
View reviewed changes
pmtk
pmtk reviewed Feb 22, 2023
View reviewed changes
pmtk
pmtk reviewed Feb 23, 2023
View reviewed changes
pmtk
pmtk reviewed Feb 23, 2023
View reviewed changes
@dusk125 dusk125 force-pushed the etcd-defrag branch 2 times, most recently from 29974e6 to 14b3b2b Compare February 23, 2023 09:18
@pmtk
Copy link
Member

pmtk commented Feb 23, 2023

I have #1404 ready - it'll fix vendoring of microshift in etcd/vendor/.
When you rebase onto that (hopefully it'll make fast into the main), changing MicroShiftConfig should be as easy as changing pkg/config/config.go and running go mod vendor in etcd/ (that file should be just copied from local dir, not downloaded from remote github.com/openshift/microshift repo)

@dusk125 dusk125 changed the title WIP: ETCD-352: Adding etcd defrag goroutine ETCD-352: Adding etcd defrag goroutine Feb 23, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 23, 2023
@dusk125
Copy link
Contributor Author

dusk125 commented Feb 27, 2023

/retest

@dusk125 dusk125 force-pushed the etcd-defrag branch 2 times, most recently from ea73e4a to aed7ec9 Compare February 28, 2023 16:13
@dusk125
Copy link
Contributor Author

dusk125 commented Feb 28, 2023

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 28, 2023 •
edited
Loading

@dusk125: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-rpm-install 98f6b8eb83e695b1cb34094026b999f67d1ea7ab link true /test e2e-rpm-install

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@pmtk
Copy link
Member

pmtk commented Mar 1, 2023

/retest

@pmtk
Copy link
Member

pmtk commented Mar 1, 2023

looks good to me
/approve

@pmtk
Copy link
Member

pmtk commented Mar 1, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 1, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 1, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dusk125, pmtk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 1, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 1, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dusk125, pmtk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 28571e4 into openshift:main Mar 1, 2023
@dusk125 dusk125 deleted the etcd-defrag branch March 1, 2023 13:50
@dusk125 dusk125 mentioned this pull request Mar 2, 2023
Merged
@dhellmann dhellmann mentioned this pull request Mar 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@copejon copejon Awaiting requested review from copejon

@oglok oglok Awaiting requested review from oglok

1 more reviewer

@pmtk pmtk pmtk left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@pmtk pmtk

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dusk125 @openshift-ci-robot @pmtk @openshift-merge-robot