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
Robustness operations failpoints #17889
Robustness operations failpoints #17889
Conversation
d884401
to
9e11172
Compare
tests/client/client.go
Outdated
@@ -12,7 +12,7 @@ | |||
// See the License for the specific language governing permissions and | |||
// limitations under the License. | |||
|
|||
package traffic | |||
package client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why moving the client out of robustness? Is it useful for other tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ups, I meant to move it to tests/robustness/client
) | ||
|
||
type trigger interface { | ||
Trigger(ctx context.Context, t *testing.T, member e2e.EtcdProcess, clus *e2e.EtcdProcessCluster) error | ||
Trigger(ctx context.Context, t *testing.T, member e2e.EtcdProcess, clus *e2e.EtcdProcessCluster, baseTime time.Time, ids identity.Provider) ([]report.ClientReport, error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some comments about when the trigger should return the clientReport? Most of the time it seems to be nil.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When it makes a request using recording client.
9e11172
to
52fac15
Compare
Signed-off-by: Marek Siarkowicz <siarkowicz@google.com>
52fac15
to
c4e3b61
Compare
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one question, but LGTM overall.
func (c *RecordingClient) Endpoints() []string { | ||
return c.client.Endpoints() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just curious when we lock and when we don't, also in the places we are using the kvMux
, we aren't appending operations unless I'm missing something. Or are we making this change proactively for some work in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We lock when we are making a call to etcd, because linearization requires that there are no concurrent operations sharing the same clientID.
cc @fuweid @siyuanfoundation @MadhavJivrajani