Yuqi Du 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 9: (1 comment) > 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"; > I'm not sure I understand -- 'tablet_schema' here points to the schema used What I reply to Yingchun is simple In fact, I use tablet_schema from beginning, but In my env testing I encoutered a coredump. I think the operator= and Reset is not the same. But I did not try to figure out the reason. Now, I checked the two functions: operator=(Schema&) and Reset(cols, ...), I found a difference, that is the code line: col_offsets_.reserve(cols_.size() + 1); // Include space for total byte size at the end. + col_offsets_.clear(); (no the line) the vector col_offsets_ is not clear, so if tablet_schema is not empty, maybe the result is not same as what we wish. I have confirmed it by a experiment. And later, I try to patch a unit test to cover it. I am not very clear the semantics of Reset(), so I am not sure whether the absence of col_offsets_.clear(); in Reset(cols, ...) is a bug, it may be a bug. -- 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: 9 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: Tue, 08 Feb 2022 03:29:38 +0000 Gerrit-HasComments: Yes
