You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org> on 2017/03/01 00:34:16 UTC

[kudu-CR] [delete table-test] Don't fail on multiple attempts to delete an orphaned tablet 

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: [delete_table-test] Don't fail on multiple attempts to delete an orphaned tablet
......................................................................

[delete_table-test] Don't fail on multiple attempts to delete an orphaned tablet

TestUnknownTabletsAreNotDeleted is flaky with the following assertion:
/data/jenkins-workspace/kudu-workspace/src/kudu/integration-tests/delete_table-test.cc:1099: Failure
Value of: num_delete_attempts
  Actual: 3
Expected: 1

This is due to the master receiving more than 1 tablet report before
the tablet server has the time to delete the tablet. Security-related
changes likely shifted the timings and the heartbeat period is set
very low in this test, so this is more likely to happen. Nonetheless
the opportunity for flakyness was there even before these changes.

In addition to fixing the assertion on the metric, this patch also
adds an assertion that the tablet was actually deleted.

Change-Id: I2b5f21c718bad8c0b7112395082371f19a61507f
---
M src/kudu/integration-tests/delete_table-test.cc
1 file changed, 12 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2b5f21c718bad8c0b7112395082371f19a61507f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>

[kudu-CR] [delete table-test] Don't fail on multiple attempts to delete an orphaned tablet

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Todd Lipcon, Kudu Jenkins,

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

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

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

Change subject: [delete_table-test] Don't fail on multiple attempts to delete an orphaned tablet
......................................................................

[delete_table-test] Don't fail on multiple attempts to delete an orphaned tablet

TestUnknownTabletsAreNotDeleted is flaky with the following assertion:
/data/jenkins-workspace/kudu-workspace/src/kudu/integration-tests/delete_table-test.cc:1099: Failure
Value of: num_delete_attempts
  Actual: 3
Expected: 1

This is due to the master receiving more than 1 tablet report before
the tablet server has the time to delete the tablet. Security-related
changes likely shifted the timings and the heartbeat period is set
very low in this test, so this is more likely to happen. Nonetheless
the opportunity for flakyness was there even before these changes.

In addition to fixing the assertion on the metric, this patch also
adds an assertion that the tablet was actually deleted.

Change-Id: I2b5f21c718bad8c0b7112395082371f19a61507f
---
M src/kudu/integration-tests/delete_table-test.cc
1 file changed, 10 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2b5f21c718bad8c0b7112395082371f19a61507f
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [delete table-test] Don't fail on multiple attempts to delete an orphaned tablet

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has uploaded a new patch set (#2).

Change subject: [delete_table-test] Don't fail on multiple attempts to delete an orphaned tablet
......................................................................

[delete_table-test] Don't fail on multiple attempts to delete an orphaned tablet

TestUnknownTabletsAreNotDeleted is flaky with the following assertion:
/data/jenkins-workspace/kudu-workspace/src/kudu/integration-tests/delete_table-test.cc:1099: Failure
Value of: num_delete_attempts
  Actual: 3
Expected: 1

This is due to the master receiving more than 1 tablet report before
the tablet server has the time to delete the tablet. Security-related
changes likely shifted the timings and the heartbeat period is set
very low in this test, so this is more likely to happen. Nonetheless
the opportunity for flakyness was there even before these changes.

In addition to fixing the assertion on the metric, this patch also
adds an assertion that the tablet was actually deleted.

Change-Id: I2b5f21c718bad8c0b7112395082371f19a61507f
---
M src/kudu/integration-tests/delete_table-test.cc
1 file changed, 10 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2b5f21c718bad8c0b7112395082371f19a61507f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [delete table-test] Don't fail on multiple attempts to delete an orphaned tablet

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has submitted this change and it was merged.

Change subject: [delete_table-test] Don't fail on multiple attempts to delete an orphaned tablet
......................................................................


[delete_table-test] Don't fail on multiple attempts to delete an orphaned tablet

TestUnknownTabletsAreNotDeleted is flaky with the following assertion:
/data/jenkins-workspace/kudu-workspace/src/kudu/integration-tests/delete_table-test.cc:1099: Failure
Value of: num_delete_attempts
  Actual: 3
Expected: 1

This is due to the master receiving more than 1 tablet report before
the tablet server has the time to delete the tablet. Security-related
changes likely shifted the timings and the heartbeat period is set
very low in this test, so this is more likely to happen. Nonetheless
the opportunity for flakyness was there even before these changes.

In addition to fixing the assertion on the metric, this patch also
adds an assertion that the tablet was actually deleted.

Change-Id: I2b5f21c718bad8c0b7112395082371f19a61507f
Reviewed-on: http://gerrit.cloudera.org:8080/6191
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: David Ribeiro Alves <dr...@apache.org>
---
M src/kudu/integration-tests/delete_table-test.cc
1 file changed, 10 insertions(+), 1 deletion(-)

Approvals:
  David Ribeiro Alves: Verified
  Adar Dembo: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I2b5f21c718bad8c0b7112395082371f19a61507f
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [delete table-test] Don't fail on multiple attempts to delete an orphaned tablet

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Todd Lipcon, Kudu Jenkins,

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

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

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

Change subject: [delete_table-test] Don't fail on multiple attempts to delete an orphaned tablet
......................................................................

[delete_table-test] Don't fail on multiple attempts to delete an orphaned tablet

TestUnknownTabletsAreNotDeleted is flaky with the following assertion:
/data/jenkins-workspace/kudu-workspace/src/kudu/integration-tests/delete_table-test.cc:1099: Failure
Value of: num_delete_attempts
  Actual: 3
Expected: 1

This is due to the master receiving more than 1 tablet report before
the tablet server has the time to delete the tablet. Security-related
changes likely shifted the timings and the heartbeat period is set
very low in this test, so this is more likely to happen. Nonetheless
the opportunity for flakyness was there even before these changes.

In addition to fixing the assertion on the metric, this patch also
adds an assertion that the tablet was actually deleted.

Change-Id: I2b5f21c718bad8c0b7112395082371f19a61507f
---
M src/kudu/integration-tests/delete_table-test.cc
1 file changed, 10 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2b5f21c718bad8c0b7112395082371f19a61507f
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [delete table-test] Don't fail on multiple attempts to delete an orphaned tablet

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: [delete_table-test] Don't fail on multiple attempts to delete an orphaned tablet
......................................................................


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2b5f21c718bad8c0b7112395082371f19a61507f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [delete table-test] Don't fail on multiple attempts to delete an orphaned tablet

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: [delete_table-test] Don't fail on multiple attempts to delete an orphaned tablet
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6191/2/src/kudu/integration-tests/delete_table-test.cc
File src/kudu/integration-tests/delete_table-test.cc:

Line 1042:   const MonoDelta timeout = MonoDelta::FromSeconds(30);
> Since this is used in just one place (L1059), could we inline it there?
Done


PS2, Line 1105: Some times
> Nit: Sometimes
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2b5f21c718bad8c0b7112395082371f19a61507f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] [delete table-test] Don't fail on multiple attempts to delete an orphaned tablet

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: [delete_table-test] Don't fail on multiple attempts to delete an orphaned tablet
......................................................................


Patch Set 4: Verified+1

unrelated flake TsRecoveryITest.TestRestartWithOrphanedReplicates

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2b5f21c718bad8c0b7112395082371f19a61507f
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [delete table-test] Don't fail on multiple attempts to delete an orphaned tablet

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: [delete_table-test] Don't fail on multiple attempts to delete an orphaned tablet
......................................................................


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2b5f21c718bad8c0b7112395082371f19a61507f
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [delete table-test] Don't fail on multiple attempts to delete an orphaned tablet

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: [delete_table-test] Don't fail on multiple attempts to delete an orphaned tablet
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6191/2/src/kudu/integration-tests/delete_table-test.cc
File src/kudu/integration-tests/delete_table-test.cc:

Line 1042:   const MonoDelta timeout = MonoDelta::FromSeconds(30);
Since this is used in just one place (L1059), could we inline it there?


PS2, Line 1105: Some times
Nit: Sometimes


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2b5f21c718bad8c0b7112395082371f19a61507f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes