Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18169 )

Change subject: [tool] fix a command bug, cmd: kudu wal dump ...
......................................................................


Patch Set 8: Code-Review+1

(1 comment)

LGTM, though would like some clarification on the thread with Yingchun

http://gerrit.cloudera.org:8080/#/c/18169/2/src/kudu/tools/tool_action_common.cc
File src/kudu/tools/tool_action_common.cc:

http://gerrit.cloudera.org:8080/#/c/18169/2/src/kudu/tools/tool_action_common.cc@358
PS2, Line 358:         LOG(ERROR) << "read an ALTER_SCHEMA_OP log entry, but 
has no alter_schema_request";
> tablet_schema is not a new object.
I'm not sure I understand -- 'tablet_schema' here points to the schema used at 
L539, and it seems that it doesn't need to be a new object to ensure the 
correct schema is used within a given segment.

I.e. why not call

 RETURN_NOT_OK(SchemaFromPB(replicate.alter_schema_request().schema(), 
tablet_schema));

which avoids this extra allocation and copy of 'schema' altogether?



--
To view, visit http://gerrit.cloudera.org:8080/18169
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27acc71597d038cafbbe687117bddb1ce16576c0
Gerrit-Change-Number: 18169
Gerrit-PatchSet: 8
Gerrit-Owner: Yuqi Du <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Reviewer: Yuqi Du <[email protected]>
Gerrit-Comment-Date: Mon, 31 Jan 2022 23:35:23 +0000
Gerrit-HasComments: Yes

Reply via email to