[impala] 02/02: IMPALA-10216: make TestWriteErrorBlacklist deterministic

[tarmstrong]

This is an automated email from the ASF dual-hosted git repository.

tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit f07514add9956e676d99ae9cbca5855960a279f2
Author: Tim Armstrong <tarmstr...@cloudera.com>
AuthorDate: Tue Nov 24 12:20:04 2020 -0800

    IMPALA-10216: make TestWriteErrorBlacklist deterministic
    
    There is a subtle bug in the test where it does a BufferPool::Pin()
    call followed immediately by a BufferPool::Unpin() call. This is
    meant to ensure that a new scratch range is allocated for the file,
    but does not guarantee that because the Pin() is asynchronous and
    there is a short-circuit case in buffer pool that cancels the
    Pin() if it the page is unpinned before the pin completes.
    
    We can force the pin to complete by requesting the actual buffer
    for the page (and verifying the data for good measure).
    
    Testing:
    I was never able to reproduce this race locally, but the fix
    is pretty safe and I looped the modified test for a while.
    
    Change-Id: I158dbc1ac60c8fefd53a138aa3e41cced9c4d674
    Reviewed-on: http://gerrit.cloudera.org:8080/16782
    Reviewed-by: Csaba Ringhofer <csringho...@cloudera.com>
    Tested-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
---
 be/src/runtime/bufferpool/buffer-pool-test.cc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/be/src/runtime/bufferpool/buffer-pool-test.cc 
b/be/src/runtime/bufferpool/buffer-pool-test.cc
index 37f21be..949ac14 100644
--- a/be/src/runtime/bufferpool/buffer-pool-test.cc
+++ b/be/src/runtime/bufferpool/buffer-pool-test.cc
@@ -1785,6 +1785,8 @@ void BufferPoolTest::TestWriteErrorBlacklist(
   DestroyAll(&pool, &clients[ERROR_QUERY], &error_new_pages);
 
   ASSERT_OK(PinAll(&pool, &clients[NO_ERROR_QUERY], &pages[NO_ERROR_QUERY]));
+  // IMPALA-10216: Verify data to force Pin to complete before unpinning.
+  VerifyData(pages[NO_ERROR_QUERY], 0);
   UnpinAll(&pool, &clients[NO_ERROR_QUERY], &pages[NO_ERROR_QUERY]);
   WaitForAllWrites(&clients[NO_ERROR_QUERY]);
   EXPECT_TRUE(FindPageInDir(pages[NO_ERROR_QUERY], good_dir) != NULL)