Adar Dembo has posted comments on this change.

Change subject: KUDU-1793: only update lbm container bookkeeping on success
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/5399/1/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

Line 598:           "Found misaligned block in data file: $0\nRecord: $1",
> Maybe add a JIRA # as explanation for people who see this message?  I would
Done


Line 755:         "Container $0 unexpectedly tried to lower its size (from $1 
to $2 "
> same here
This one is actually just defensive programming or future proofing, if for some 
reason block records show up out of order. I don't expect that to happen, but I 
felt uncomfortable using a DCHECK/CHECK here.


Line 1605:             "Could not process record in container $o", 
container->ToString()));
> that looks like an o and not a 0?
Whoops. Good catch!


Line 1689:       LOG(WARNING) << "Skipping unknown op type in block record: "
> This could be a forwards compatibility hazard.  Is there a strong reason no
The code as it was before prevents us from adding new record types and allowing 
downgrades. I thought we generally like supporting downgrades?

But if you think it's too dangerous, I'll revert it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I49bc98c9f8b7dce0333f88cec85757fe122acfa4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to