impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-3201: in-memory buffer pool implementation
Date Tue, 27 Sep 2016 23:06:15 GMT
Dan Hecht has posted comments on this change.

Change subject: IMPALA-3201: in-memory buffer pool implementation

Patch Set 7:

File be/src/bufferpool/

PS7, Line 66:  > 0
delete (though this may become pin_count anyway).

PS7, Line 276: page->buffer.is_open()
why not page->pinned?  Would be equivalent but seems more direct in intent.
File be/src/bufferpool/buffer-pool.h:

PS7, Line 262:  

Line 382:   int pin_count() const { return pin_count_; }
why do we need to expose this?

Line 414:   void DecrementPinCount();
if we get rid of pin_count_ in the handle and only store that in the page (see comment below),
then these wouldn't be needed and it would be one less abstraction level to think about.

Line 421:   const Client* client_;
let's consider not having this as it doesn't seem to add much value. we can always add it
later if necessary.

Line 429:   const BufferHandle* buffer_handle_;
this would also go away.

Line 432:   int pin_count_;
why do we have this and the pinned_ boolean in the page?  i.e why not just have a single count
in the page and no state in the handle?

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I4bda61c31cc02d26bc83c3d458c835b0984b86a0
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-HasComments: Yes

View raw message