You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2020/09/16 19:56:14 UTC

[kudu-CR] test: fix file block manager tests following eaf1b6d

Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16460


Change subject: test: fix file block manager tests following eaf1b6d
......................................................................

test: fix file block manager tests following eaf1b6d

The failures were mostly caused by differences in error messages in
startup failure scenarios following the refactoring in commit eaf1b6d.

There was also a functional test adjustment: a canonicalization failure
will  allow a FBM-backed server to start up with a failed directory,
rather than failing startup altogethr.

Another test for adding and removing directories is disabled entirely,
as this is not supported by the file block manager.

This also updates fs_manager-test to run using both the file block
manager and log block manager.

Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
---
M src/kudu/fs/fs_manager-test.cc
M src/kudu/tserver/tablet_server-test.cc
2 files changed, 50 insertions(+), 48 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
Gerrit-Change-Number: 16460
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>

[kudu-CR] test: fix file block manager tests following eaf1b6d

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

Change subject: test: fix file block manager tests following eaf1b6d
......................................................................


Patch Set 4: Verified+1

Not sure why there was no post back here, but all the tests passed: http://jenkins.kudu.apache.org/job/kudu-gerrit/22419/


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
Gerrit-Change-Number: 16460
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 23:04:28 +0000
Gerrit-HasComments: No

[kudu-CR] test: fix file block manager tests following eaf1b6d

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

Change subject: test: fix file block manager tests following eaf1b6d
......................................................................


Patch Set 4: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
Gerrit-Change-Number: 16460
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 22:35:04 +0000
Gerrit-HasComments: No

[kudu-CR] test: fix file block manager tests following eaf1b6d

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

Change subject: test: fix file block manager tests following eaf1b6d
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16460/2/src/kudu/fs/fs_manager-test.cc
File src/kudu/fs/fs_manager-test.cc:

http://gerrit.cloudera.org:8080/#/c/16460/2/src/kudu/fs/fs_manager-test.cc@457
PS2, Line 457:   // NOTE: the failure case looks slightly different depending on the block
Is it challenging to unify them?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
Gerrit-Change-Number: 16460
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 20:38:12 +0000
Gerrit-HasComments: Yes

[kudu-CR] test: fix file block manager tests following eaf1b6d

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Grant Henke, 

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

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

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

Change subject: test: fix file block manager tests following eaf1b6d
......................................................................

test: fix file block manager tests following eaf1b6d

The failures were mostly caused by differences in error messages in
startup failure scenarios following the refactoring in commit eaf1b6d.

There was also a functional test adjustment: a canonicalization failure
will allow a FBM-backed server to start up with a failed directory,
rather than failing startup altogether.

Another test for adding and removing directories is disabled entirely,
as this is not supported by the file block manager.

This also updates fs_manager-test to run using both the file block
manager and log block manager.

Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
---
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/fs_manager-test.cc
M src/kudu/tserver/tablet_server-test.cc
3 files changed, 54 insertions(+), 50 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
Gerrit-Change-Number: 16460
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] test: fix file block manager tests following eaf1b6d

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

Change subject: test: fix file block manager tests following eaf1b6d
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16460/2/src/kudu/fs/fs_manager-test.cc
File src/kudu/fs/fs_manager-test.cc:

http://gerrit.cloudera.org:8080/#/c/16460/2/src/kudu/fs/fs_manager-test.cc@457
PS2, Line 457:   // NOTE: the failure case looks slightly different depending on the block
> Is it challenging to unify them?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
Gerrit-Change-Number: 16460
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 21:04:41 +0000
Gerrit-HasComments: Yes

[kudu-CR] test: fix file block manager tests following eaf1b6d

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

Change subject: test: fix file block manager tests following eaf1b6d
......................................................................


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
Gerrit-Change-Number: 16460
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 20:11:08 +0000
Gerrit-HasComments: No

[kudu-CR] test: fix file block manager tests following eaf1b6d

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Grant Henke, 

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

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

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

Change subject: test: fix file block manager tests following eaf1b6d
......................................................................

test: fix file block manager tests following eaf1b6d

The failures were mostly caused by differences in error messages in
startup failure scenarios following the refactoring in commit eaf1b6d.

There was also a functional test adjustment: a canonicalization failure
will allow a FBM-backed server to start up with a failed directory,
rather than failing startup altogether.

Another test for adding and removing directories is disabled entirely,
as this is not supported by the file block manager.

This also updates fs_manager-test to run using both the file block
manager and log block manager.

Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
---
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/fs_manager-test.cc
M src/kudu/tserver/tablet_server-test.cc
3 files changed, 54 insertions(+), 50 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
Gerrit-Change-Number: 16460
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] test: fix file block manager tests following eaf1b6d

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Grant Henke, 

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

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

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

Change subject: test: fix file block manager tests following eaf1b6d
......................................................................

test: fix file block manager tests following eaf1b6d

The failures were mostly caused by differences in error messages in
startup failure scenarios following the refactoring in commit eaf1b6d.

There was also a functional test adjustment: a canonicalization failure
will allow a FBM-backed server to start up with a failed directory,
rather than failing startup altogether.

Another test for adding and removing directories is disabled entirely,
as this is not supported by the file block manager.

This also updates fs_manager-test to run using both the file block
manager and log block manager.

Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
---
M src/kudu/fs/fs_manager-test.cc
M src/kudu/tserver/tablet_server-test.cc
2 files changed, 57 insertions(+), 49 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
Gerrit-Change-Number: 16460
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] test: fix file block manager tests following eaf1b6d

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

Change subject: test: fix file block manager tests following eaf1b6d
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
Gerrit-Change-Number: 16460
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 21:10:59 +0000
Gerrit-HasComments: No

[kudu-CR] test: fix file block manager tests following eaf1b6d

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

Change subject: test: fix file block manager tests following eaf1b6d
......................................................................

test: fix file block manager tests following eaf1b6d

The failures were mostly caused by differences in error messages in
startup failure scenarios following the refactoring in commit eaf1b6d.

There was also a functional test adjustment: a canonicalization failure
will allow a FBM-backed server to start up with a failed directory,
rather than failing startup altogether.

Another test for adding and removing directories is disabled entirely,
as this is not supported by the file block manager.

This also updates fs_manager-test to run using both the file block
manager and log block manager.

Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
Reviewed-on: http://gerrit.cloudera.org:8080/16460
Reviewed-by: Grant Henke <gr...@apache.org>
Tested-by: Kudu Jenkins
Tested-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/fs_manager-test.cc
M src/kudu/tserver/tablet_server-test.cc
3 files changed, 54 insertions(+), 50 deletions(-)

Approvals:
  Grant Henke: Looks good to me, approved
  Kudu Jenkins: Verified
  Andrew Wong: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
Gerrit-Change-Number: 16460
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] test: fix file block manager tests following eaf1b6d

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

Change subject: test: fix file block manager tests following eaf1b6d
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16460/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16460/3//COMMIT_MSG@12
PS3, Line 12: There was also a functional test adjustment
> Is it still so after this change?  If so, is it make sense to have the beha
It is: https://gerrit.cloudera.org/c/16460/3/src/kudu/fs/fs_manager-test.cc#616

It's a test adjustment to reflect what actually occurs, and I think the behavior is reasonable given it's similar behavior to the log block manager, so I don't think I'll update it further.


http://gerrit.cloudera.org:8080/#/c/16460/3/src/kudu/fs/fs_manager-test.cc
File src/kudu/fs/fs_manager-test.cc:

http://gerrit.cloudera.org:8080/#/c/16460/3/src/kudu/fs/fs_manager-test.cc@463
PS3, Line 463: s.ToString(), "i
> nit: this wouldn't output much useful information if the assert triggers (w
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
Gerrit-Change-Number: 16460
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 22:05:44 +0000
Gerrit-HasComments: Yes

[kudu-CR] test: fix file block manager tests following eaf1b6d

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

Change subject: test: fix file block manager tests following eaf1b6d
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16460/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16460/3//COMMIT_MSG@12
PS3, Line 12: There was also a functional test adjustment
Is it still so after this change?  If so, is it make sense to have the behavior more uniform (if it's not too hard)?


http://gerrit.cloudera.org:8080/#/c/16460/3/src/kudu/fs/fs_manager-test.cc
File src/kudu/fs/fs_manager-test.cc:

http://gerrit.cloudera.org:8080/#/c/16460/3/src/kudu/fs/fs_manager-test.cc@463
PS3, Line 463:  << s.ToString()
nit: this wouldn't output much useful information if the assert triggers (would be just 'OK').



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b210869464c69d0ff024d030bd553d23b21352f
Gerrit-Change-Number: 16460
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 16 Sep 2020 21:38:33 +0000
Gerrit-HasComments: Yes