[Impala-CR](cdh5-trunk) IMPALA-3201: HEADER ONLY: buffer pool
Tim Armstrong has uploaded a new patch set (#3). Change subject: IMPALA-3201: HEADER ONLY: buffer pool .. IMPALA-3201: HEADER ONLY: buffer pool Change-Id: Id771dea2eb4c1aa13c30d59e8b184a7d1bca8d34 --- A be/src/bufferpool/buffer-pool.h 1 file changed, 326 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/88/3688/3 -- To view, visit http://gerrit.cloudera.org:8080/3688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id771dea2eb4c1aa13c30d59e8b184a7d1bca8d34 Gerrit-PatchSet: 3 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-CR](cdh5-trunk) IMPALA-3201: HEADER ONLY: buffer pool page-only interface
Tim Armstrong has abandoned this change. Change subject: IMPALA-3201: HEADER ONLY: buffer pool page-only interface .. Abandoned -- To view, visit http://gerrit.cloudera.org:8080/3762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I9698d3a0b2be4f8963d0174bcf383838acc0c925 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Michael Ho
[Impala-CR](cdh5-trunk) IMPALA-3201: HEADER ONLY: buffer pool
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3201: HEADER ONLY: buffer pool .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/3688/2/be/src/bufferpool/buffer-pool.h File be/src/bufferpool/buffer-pool.h: Line 303: uint8_t* buf() const; > PageHandle should not have this function. all access of the underlying data I don't understand the problem with this if you're proposing that the page owns the buffer. page->buffer()->buffer() or whatever the alternative is seems overly fussy. -- To view, visit http://gerrit.cloudera.org:8080/3688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id771dea2eb4c1aa13c30d59e8b184a7d1bca8d34 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3201: HEADER ONLY: buffer pool
Marcel Kornacker has posted comments on this change. Change subject: IMPALA-3201: HEADER ONLY: buffer pool .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/3688/2/be/src/bufferpool/buffer-pool.h File be/src/bufferpool/buffer-pool.h: Line 303: uint8_t* buf() const; PageHandle should not have this function. all access of the underlying data needs to go through the buffer abstraction (otherwise you don't really have a separation of abstractions). algorithms that only implement data processing logic should only deal with buffers, whereas algorithms that only implement spilling-type logic should only deal with pages. -- To view, visit http://gerrit.cloudera.org:8080/3688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id771dea2eb4c1aa13c30d59e8b184a7d1bca8d34 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3201: HEADER ONLY: buffer pool
Dan Hecht has posted comments on this change. Change subject: IMPALA-3201: HEADER ONLY: buffer pool .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/3688/2/be/src/bufferpool/buffer-pool.h File be/src/bufferpool/buffer-pool.h: Line 219: void DuplicateHandle(const PageHandle& src, Client* client, PageHandle* dst); Doesn't seem like we need this. Line 296: BufferHandle ExtractBuffer(); It'd be nice for this to live side-by-side with Pin() and Unpin() so either move this to BufferPool or move those Page operations here. My slight preference is to move this and leave Buffer/Page more like structs since the buffer pool is the thing that has to do the heavy lifting, but I'm okay either way. -- To view, visit http://gerrit.cloudera.org:8080/3688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id771dea2eb4c1aa13c30d59e8b184a7d1bca8d34 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3201: HEADER ONLY: buffer pool
Dan Hecht has posted comments on this change. Change subject: IMPALA-3201: HEADER ONLY: buffer pool .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/3688/2/be/src/bufferpool/buffer-pool.h File be/src/bufferpool/buffer-pool.h: Line 227: PageHandle* dst); is this needed still? wouldn't we only be transferring buffers via TransferBuffer()? -- To view, visit http://gerrit.cloudera.org:8080/3688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id771dea2eb4c1aa13c30d59e8b184a7d1bca8d34 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3201: HEADER ONLY: buffer pool
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3201: HEADER ONLY: buffer pool .. Patch Set 2: Updated with buffer-oriented naming and private members stripped out. For comparison, I posted the same interface with only the page methods exposed: https://gerrit.cloudera.org/#/c/3762/ -- To view, visit http://gerrit.cloudera.org:8080/3688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id771dea2eb4c1aa13c30d59e8b184a7d1bca8d34 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3201: HEADER ONLY: buffer pool page-only interface
Tim Armstrong has uploaded a new change for review. http://gerrit.cloudera.org:8080/3762 Change subject: IMPALA-3201: HEADER ONLY: buffer pool page-only interface .. IMPALA-3201: HEADER ONLY: buffer pool page-only interface This is the header only patch with only the page interface exposed. Change-Id: I9698d3a0b2be4f8963d0174bcf383838acc0c925 --- A be/src/bufferpool/buffer-pool.h 1 file changed, 301 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/62/3762/1 -- To view, visit http://gerrit.cloudera.org:8080/3762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9698d3a0b2be4f8963d0174bcf383838acc0c925 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong
[Impala-CR](cdh5-trunk) IMPALA-3201: HEADER ONLY: buffer pool
Tim Armstrong has uploaded a new patch set (#2). Change subject: IMPALA-3201: HEADER ONLY: buffer pool .. IMPALA-3201: HEADER ONLY: buffer pool Change-Id: Id771dea2eb4c1aa13c30d59e8b184a7d1bca8d34 --- A be/src/bufferpool/buffer-pool.h 1 file changed, 336 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/88/3688/2 -- To view, visit http://gerrit.cloudera.org:8080/3688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id771dea2eb4c1aa13c30d59e8b184a7d1bca8d34 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-CR](cdh5-trunk) IMPALA-3201: HEADER ONLY: buffer pool
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3201: HEADER ONLY: buffer pool .. Patch Set 1: One point that I alluded to in the previous comment is that passing an InMemPageHandle allows compile-time checking of whether the page is pinned, but doesn't provide compile-time checking of whether the handle is closed. -- To view, visit http://gerrit.cloudera.org:8080/3688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id771dea2eb4c1aa13c30d59e8b184a7d1bca8d34 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3201: HEADER ONLY: buffer pool
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3201: HEADER ONLY: buffer pool .. Patch Set 1: This is an updated header where was have a new InMemPageHandle abstraction that guarantees that the referenced page is in memory (provided the handle is open). I've posted the header only so we can agree on the interface before diving into the implementation. -- To view, visit http://gerrit.cloudera.org:8080/3688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id771dea2eb4c1aa13c30d59e8b184a7d1bca8d34 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3201: HEADER ONLY: buffer pool
Tim Armstrong has uploaded a new change for review. http://gerrit.cloudera.org:8080/3688 Change subject: IMPALA-3201: HEADER ONLY: buffer pool .. IMPALA-3201: HEADER ONLY: buffer pool Change-Id: Id771dea2eb4c1aa13c30d59e8b184a7d1bca8d34 --- A be/src/bufferpool/buffer-pool.h 1 file changed, 422 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/88/3688/1 -- To view, visit http://gerrit.cloudera.org:8080/3688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id771dea2eb4c1aa13c30d59e8b184a7d1bca8d34 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong
