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 2019/01/05 01:25:20 UTC

[kudu-CR] KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks

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


Change subject: KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks
......................................................................

KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks

The test waits for a write workload to generate some orphaned blocks.
With the default write pattern, this could sometimes take a while and
cause the test to hit an error. This patch makes orphaned block
generation faster by making the workload update a single row.

Before this, I looped the test in debug mode with 32 stress threads and
it failed 15/100 times. With the fix, this passed 1000/1000.

Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 1 insertion(+), 0 deletions(-)



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

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

[kudu-CR] KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks

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

Change subject: KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c
Gerrit-Change-Number: 12166
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Wed, 09 Jan 2019 00:23:33 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks

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

Change subject: KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks
......................................................................

KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks

The test waits for a write workload to generate some orphaned blocks.
With the default write pattern, this could sometimes take a while and
cause the test to hit an error. This patch makes orphaned block
generation faster by making the workload update a single row.

Before this, I looped the test in debug mode with 32 stress threads and
it failed 15/100 times. With the fix, this passed 1000/1000.

Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c
Reviewed-on: http://gerrit.cloudera.org:8080/12166
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Mike Percy <mp...@apache.org>
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 3 insertions(+), 0 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c
Gerrit-Change-Number: 12166
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks

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

Change subject: KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12166/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12166/1//COMMIT_MSG@11
PS1, Line 11: This patch makes orphaned block
            : generation faster by making the workload update a single row.
> Hmm, why is this obviously faster? Updating one row over and over means we'
I would also suspect Will's compaction changes; I wonder if flushing outstrips the merge compactions in the old scenario now.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c
Gerrit-Change-Number: 12166
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 08 Jan 2019 23:40:57 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks

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

Change subject: KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12166/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12166/1//COMMIT_MSG@11
PS1, Line 11: This patch makes orphaned block
            : generation faster by making the workload update a single row.
> I would also suspect Will's compaction changes; I wonder if flushing outstr
>Hmm, why is this obviously faster?
The default is a random write workload, no a sequential one. It's a bit harder to reason about what the rowsets look like, but I'd expect either sequential writes or single-row-updates to definitely generate orphaned blocks.

>I also wonder why this surfaced just now.
I actually just saw this fail in a Cloudera-internal 1.8 build, so I don't think the flakiness was introduced by the recent compaction policy changes.


http://gerrit.cloudera.org:8080/#/c/12166/1/src/kudu/integration-tests/ts_recovery-itest.cc
File src/kudu/integration-tests/ts_recovery-itest.cc:

http://gerrit.cloudera.org:8080/#/c/12166/1/src/kudu/integration-tests/ts_recovery-itest.cc@257
PS1, Line 257:     // We want to generate orphaned blocks, so use a workload that we expect to
> I think the explanation is that we end up generating lots of (major and/or 
Done, yes.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c
Gerrit-Change-Number: 12166
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 08 Jan 2019 23:48:40 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks

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

Change subject: KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c
Gerrit-Change-Number: 12166
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Wed, 09 Jan 2019 00:45:53 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks

Posted by "Andrew Wong (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/12166

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

Change subject: KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks
......................................................................

KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks

The test waits for a write workload to generate some orphaned blocks.
With the default write pattern, this could sometimes take a while and
cause the test to hit an error. This patch makes orphaned block
generation faster by making the workload update a single row.

Before this, I looped the test in debug mode with 32 stress threads and
it failed 15/100 times. With the fix, this passed 1000/1000.

Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 3 insertions(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c
Gerrit-Change-Number: 12166
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks

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

Change subject: KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12166/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12166/1//COMMIT_MSG@11
PS1, Line 11: This patch makes orphaned block
            : generation faster by making the workload update a single row.
Hmm, why is this obviously faster? Updating one row over and over means we'll flush one MRS (INSERT and n UPDATEs), then flushing that DRS's DMS over and over. I guess there'll be some delta compaction which will delete blocks, but is that de facto more likely than the merge compaction you'd expect to see on the tiny DRSes produced by the default write pattern?

I also wonder why this surfaced just now. I suspect Will's change to the merge compaction policy, but given the non-default values to flush_threshold_mb and flush_threshold_secs I would have expected the change to make merge compaction more likely not less.


http://gerrit.cloudera.org:8080/#/c/12166/1/src/kudu/integration-tests/ts_recovery-itest.cc
File src/kudu/integration-tests/ts_recovery-itest.cc:

http://gerrit.cloudera.org:8080/#/c/12166/1/src/kudu/integration-tests/ts_recovery-itest.cc@257
PS1, Line 257:     write_workload->set_write_pattern(TestWorkload::UPDATE_ONE_ROW);
Should add a comment here explaining why we're using this.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c
Gerrit-Change-Number: 12166
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Sat, 05 Jan 2019 02:01:52 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks

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

Change subject: KUDU-2652: deflake TsRecoveryITest.TestNoBlockIDReuseIfMissingBlocks
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12166/1/src/kudu/integration-tests/ts_recovery-itest.cc
File src/kudu/integration-tests/ts_recovery-itest.cc:

http://gerrit.cloudera.org:8080/#/c/12166/1/src/kudu/integration-tests/ts_recovery-itest.cc@257
PS1, Line 257:     write_workload->set_write_pattern(TestWorkload::UPDATE_ONE_ROW);
> Should add a comment here explaining why we're using this.
I think the explanation is that we end up generating lots of (major and/or minor) delta compactions, right?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I24a689702d29a744be8c113fedafec0307a90b1c
Gerrit-Change-Number: 12166
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 08 Jan 2019 23:39:23 +0000
Gerrit-HasComments: Yes