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

storage: replace DeepEqual in (*EvalResult) MergeAndDestroy #16591

Closed
tbg opened this issue Jun 19, 2017 · 5 comments
Closed

storage: replace DeepEqual in (*EvalResult) MergeAndDestroy #16591

tbg opened this issue Jun 19, 2017 · 5 comments
Assignees
Labels
E-easy Easy issue to tackle, requires little or no CockroachDB experience help wanted Help is requested / needed by the one who filed the issue to fix it.

Comments

@tbg
Copy link
Member

tbg commented Jun 19, 2017

DeepEqual is slow and it should be replaced by a new method (*EvalResult).IsZero (which has an appropriate test and won't rot as we add fields to EvalResult or its substructs).

@tbg tbg added E-easy Easy issue to tackle, requires little or no CockroachDB experience help wanted Help is requested / needed by the one who filed the issue to fix it. labels Jun 19, 2017
@a6802739
Copy link
Contributor

@tschottdorf I could do this.

@tbg
Copy link
Member Author

tbg commented Jun 19, 2017

Thanks @a6802739, all yours!

@a6802739 a6802739 self-assigned this Jun 19, 2017
@bdarnell
Copy link
Contributor

We could use the gogoproto.equal option to generate a comparison function for ReplicatedEvalResult. Then we'd only need a hand-written IsZero function for the EvalResult struct (which only has three fields).

Alternately, the reason we can't use the == operator for ReplicatedEvalResult is the fact that it contains start_key and end_key fields (of type roachpb.Key). These fields also take up a lot of space, as reported in #16075. If we could figure out a way to get rid of these fields (perhaps replacing them with a range descriptor sequence number as mentioned in #16075 (comment)), we could go back to using ==.

@a6802739
Copy link
Contributor

@bdarnell , how could I use protoc command to generate *.pb.go file for *.proto file?

@jordanlewis
Copy link
Member

@a6802739, we have a Makefile target for that: make protobuf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Easy issue to tackle, requires little or no CockroachDB experience help wanted Help is requested / needed by the one who filed the issue to fix it.
Projects
None yet
Development

No branches or pull requests

4 participants