You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Anupama Gupta (Code Review)" <ge...@cloudera.org> on 2018/05/24 21:20:33 UTC

[kudu-CR] KUDU-702 Add block IDs to more log messages 

Anupama Gupta has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10505


Change subject: KUDU-702 Add block IDs to more log messages
......................................................................

KUDU-702 Add block IDs to more log messages

Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/index_btree.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/tserver/tablet_copy_source_session.cc
4 files changed, 9 insertions(+), 7 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/10505/1
-- 
To view, visit http://gerrit.cloudera.org:8080/10505
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 1
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>

[kudu-CR] KUDU-702 Add block IDs to more log messages

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10505 )

Change subject: KUDU-702 Add block IDs to more log messages
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10505/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10505/4//COMMIT_MSG@13
PS4, Line 13:  
nit: extra space



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 4
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 29 May 2018 19:46:33 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-702 Add block IDs to more log messages

Posted by "Anupama Gupta (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Alexey Serbin, Kudu Jenkins, Adar Dembo, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10505

to look at the new patch set (#5).

Change subject: KUDU-702 Add block IDs to more log messages
......................................................................

KUDU-702 Add block IDs to more log messages

Added block id to the log messages in the following cases:
1. Log Warnings during cfile read
2. VLOG on adding and removing unlocked blocks

Testing was performed by manually inspecting the log message
outputs generated by executing the affected statements.

Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/index_btree.cc
M src/kudu/fs/log_block_manager.cc
3 files changed, 9 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/10505/5
-- 
To view, visit http://gerrit.cloudera.org:8080/10505
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 5
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-702 Add block IDs to more log messages

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10505 )

Change subject: KUDU-702 Add block IDs to more log messages
......................................................................


Patch Set 1:

(4 comments)

Thanks for the patch! See inline for comments. Could use more detail in the commit message as well as a comment about how this was tested. For log messages, manual testing by running some of the tests and manually checking that the log messages were printed as expected is usually how we do it.

Maybe log_block_manager-test would be a good test to check for the messages.

http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/cfile/cfile_reader.cc
File src/kudu/cfile/cfile_reader.cc:

http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/cfile/cfile_reader.cc@491
PS1, Line 491:       LOG(WARNING) << "Unable to validate compressed block at "
             :                    << ptr.offset() << " with id " << block_id().ToString()
             :                    << " of size " << block.size() << ": "
             :                    << s.ToString();
Nit: the ordering here seems off to me, I think we should first identify the block id and then print the offset and size, since offset and size are logically the two parts you need for a pointer.

Same with the below change around line 510.


http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/cfile/index_btree.cc
File src/kudu/cfile/index_btree.cc:

http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/cfile/index_btree.cc@109
PS1, Line 109:     LOG(ERROR) << "Unable to flush root index block" << ptr.ToString();
This is not likely to work as intended because a non-OK Status was returned from FinishAndWriteBlock(), resulting in the out-parameter 'ptr' being left in an undefined state (probably untouched).

Actually in this case I think we should remove the ERROR log and instead put the same information in the return Status object like:

  return s.CloneAndPrepend("Unable to flush root index block");


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

http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/fs/log_block_manager.cc@2041
PS1, Line 2041:   VLOG(2) << Substitute("Added block: offset $0, length $1, id $2",
nit: for consistency i have the same suggestion for ordering in this file (here and below) as in the other file


http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/tserver/tablet_copy_source_session.cc
File src/kudu/tserver/tablet_copy_source_session.cc:

http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/tserver/tablet_copy_source_session.cc@395
PS1, Line 395:     LOG(DFATAL) << "Data block " << block_id.ToString() << " disappeared: " << s.ToString();
This isn't necessary because we already included it in the error message on the line above



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 1
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Thu, 24 May 2018 22:00:31 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-702 Add block IDs to more log messages

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10505 )

Change subject: KUDU-702 Add block IDs to more log messages
......................................................................


Patch Set 4:

Sure, this seems like an improvement as is. My original JIRA filing wasn't very specific so I'm not sure exactly what I was hoping for when I filed it, but seems we can call it closed and open a new one if we have a specific request.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 4
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 29 May 2018 19:46:01 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-702 Add block IDs to more log messages

Posted by "Anupama Gupta (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Kudu Jenkins, Adar Dembo, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10505

to look at the new patch set (#4).

Change subject: KUDU-702 Add block IDs to more log messages
......................................................................

KUDU-702 Add block IDs to more log messages

Added block id to the log messages in the following cases:
1. Log Warnings during cfile read
2. VLOG on adding and removing unlocked blocks

Testing was performed by manually inspecting the log message 
outputs generated by executing the affected statements.

Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/index_btree.cc
M src/kudu/fs/log_block_manager.cc
3 files changed, 9 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/10505/4
-- 
To view, visit http://gerrit.cloudera.org:8080/10505
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 4
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-702 Add block IDs to more log messages

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10505 )

Change subject: KUDU-702 Add block IDs to more log messages
......................................................................

KUDU-702 Add block IDs to more log messages

Added block id to the log messages in the following cases:
1. Log Warnings during cfile read
2. VLOG on adding and removing unlocked blocks

Testing was performed by manually inspecting the log message
outputs generated by executing the affected statements.

Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Reviewed-on: http://gerrit.cloudera.org:8080/10505
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/index_btree.cc
M src/kudu/fs/log_block_manager.cc
3 files changed, 9 insertions(+), 9 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Mike Percy: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 6
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-702 Add block IDs to more log messages

Posted by "Anupama Gupta (Code Review)" <ge...@cloudera.org>.
Anupama Gupta has posted comments on this change. ( http://gerrit.cloudera.org:8080/10505 )

Change subject: KUDU-702 Add block IDs to more log messages
......................................................................


Patch Set 2:

> I don't see how this patch addresses KUDU-702, because all of the
 > additions here are for edge cases that generate WARNING or ERROR
 > logging, whereas KUDU-702 asks for additions to INFO-level logging.
 > 
 > One of Todd's comments in KUDU-702 suggests that maybe a lot of the
 > existing candidates for improving are now gone. Is that the case?
 > If so, what is KUDU-702 now trying to achieve, and how does this
 > patch help achieve that?

Thats right. I could not find any INFO logs to add block ids to. In case you have any suggestions where I could further add this information please let me know. Thanks !


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 2
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 25 May 2018 18:19:42 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-702 Add block IDs to more log messages

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10505 )

Change subject: KUDU-702 Add block IDs to more log messages
......................................................................


Patch Set 3: Code-Review+1

Will defer to Mike.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 3
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 25 May 2018 21:26:39 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-702 Add block IDs to more log messages

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10505 )

Change subject: KUDU-702 Add block IDs to more log messages
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 5
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 29 May 2018 23:06:00 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-702 Add block IDs to more log messages

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10505 )

Change subject: KUDU-702 Add block IDs to more log messages
......................................................................


Patch Set 5:

Thanks for looking guys, I am going to commit this now.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 5
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 29 May 2018 23:06:18 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-702 Add block IDs to more log messages

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10505 )

Change subject: KUDU-702 Add block IDs to more log messages
......................................................................


Patch Set 4:

This LGTM, worth checking with Todd to see if he can think of anything off the top of his head and we should add to this. Either way we can then call the JIRA resolved as well.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 4
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 29 May 2018 07:17:37 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-702 Add block IDs to more log messages

Posted by "Anupama Gupta (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Kudu Jenkins, Adar Dembo, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10505

to look at the new patch set (#3).

Change subject: KUDU-702 Add block IDs to more log messages
......................................................................

KUDU-702 Add block IDs to more log messages

Added block id to the log messages in the following cases:
1. Log Warnings during cfile read
2. VLOG on adding and removing unlocked blocks

Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/index_btree.cc
M src/kudu/fs/log_block_manager.cc
3 files changed, 9 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/10505/3
-- 
To view, visit http://gerrit.cloudera.org:8080/10505
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 3
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-702 Add block IDs to more log messages

Posted by "Anupama Gupta (Code Review)" <ge...@cloudera.org>.
Anupama Gupta has posted comments on this change. ( http://gerrit.cloudera.org:8080/10505 )

Change subject: KUDU-702 Add block IDs to more log messages
......................................................................


Patch Set 4:

(4 comments)

Please review for submission.

http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/cfile/cfile_reader.cc
File src/kudu/cfile/cfile_reader.cc:

http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/cfile/cfile_reader.cc@491
PS1, Line 491:       LOG(WARNING) << "Unable to validate compressed block " << block_id().ToString()
             :                    << " at " << ptr.offset() << " of size " << block.size() << ": "
             :                    << s.ToString();
             :       return s;
> Nit: the ordering here seems off to me, I think we should first identify th
Done


http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/cfile/index_btree.cc
File src/kudu/cfile/index_btree.cc:

http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/cfile/index_btree.cc@109
PS1, Line 109:     return s.CloneAndPrepend("Unable to flush root index block");
> This is not likely to work as intended because a non-OK Status was returned
Done


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

http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/fs/log_block_manager.cc@2041
PS1, Line 2041:   VLOG(2) << Substitute("Added block: id $0, offset $1, length $2",
> nit: for consistency i have the same suggestion for ordering in this file (
Done


http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/tserver/tablet_copy_source_session.cc
File src/kudu/tserver/tablet_copy_source_session.cc:

http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/tserver/tablet_copy_source_session.cc@395
PS1, Line 395:     LOG(DFATAL) << "Data block disappeared: " << s.ToString();
> This isn't necessary because we already included it in the error message on
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 4
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Sat, 26 May 2018 03:44:16 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-702 Add block IDs to more log messages

Posted by "Anupama Gupta (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Kudu Jenkins, Adar Dembo, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10505

to look at the new patch set (#2).

Change subject: KUDU-702 Add block IDs to more log messages
......................................................................

KUDU-702 Add block IDs to more log messages

Added block id to the log messages in the following cases:
1. Log Warnings during cfile read
2. VLOG on adding and removing unlocked blocks

Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/index_btree.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/tserver/tablet_copy_source_session.cc
4 files changed, 9 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/10505/2
-- 
To view, visit http://gerrit.cloudera.org:8080/10505
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 2
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-702 Add block IDs to more log messages

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10505 )

Change subject: KUDU-702 Add block IDs to more log messages
......................................................................


Patch Set 1:

I don't see how this patch addresses KUDU-702, because all of the additions here are for edge cases that generate WARNING or ERROR logging, whereas KUDU-702 asks for additions to INFO-level logging.

One of Todd's comments in KUDU-702 suggests that maybe a lot of the existing candidates for improving are now gone. Is that the case? If so, what is KUDU-702 now trying to achieve, and how does this patch help achieve that?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 1
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 25 May 2018 17:11:39 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-702 Add block IDs to more log messages

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10505 )

Change subject: KUDU-702 Add block IDs to more log messages
......................................................................


Patch Set 4: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/cfile/cfile_reader.cc
File src/kudu/cfile/cfile_reader.cc:

http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/cfile/cfile_reader.cc@491
PS1, Line 491:       LOG(WARNING) << "Unable to validate compressed block " << block_id().ToString()
             :                    << " at " << ptr.offset() << " of size " << block.size() << ": "
             :                    << s.ToString();
             :       return s;
> Nit: the ordering here seems off to me, I think we should first identify th
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d
Gerrit-Change-Number: 10505
Gerrit-PatchSet: 4
Gerrit-Owner: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Anupama Gupta <an...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 29 May 2018 07:16:12 +0000
Gerrit-HasComments: Yes