impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4835: Part 1: simplify I/O mgr mem mgmt
Date Wed, 08 Nov 2017 23:06:55 GMT
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8414 )

Change subject: IMPALA-4835: Part 1: simplify I/O mgr mem mgmt
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8414/4/be/src/runtime/disk-io-mgr.cc
File be/src/runtime/disk-io-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/8414/4/be/src/runtime/disk-io-mgr.cc@797
PS4, Line 797:       reader->disk_states_[disk_queue->disk_id].DecrementRequestThread();
> I spent some more time looking at this and I'm increasingly thinking that t
So I understand the rule now. Once the DiskIoRequestContext is cancelled, the last thread
to leave the building must turn off the lights. If it's cancelled you need to call DecrementRequestThreadAndCheckDone().
If it's not cancelled,  you need to call DecrementRequestThread(). Both functions assume that
the caller holds lock_, so there's no reason we couldn't just combine the functions and check
the status.

The reason that HandleReadFinished() works is that it checks for the request context being
cancelled earlier in the critical section and exits early.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If5cb42437d11c13bc4a55c3ab426b66777332bd1
Gerrit-Change-Number: 8414
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <twang@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Nov 2017 23:06:55 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message