pg_buffercache tap test for buffer resize#6
Conversation
| # Wake up buffercache scan | ||
| $node->safe_psql('postgres', "SELECT injection_points_wakeup('$buffercache_injection_point')"); | ||
|
|
||
| $buffercache_session->query(q(\echo 'done')); |
There was a problem hiding this comment.
Any chance we can receive the output of the buffer scan query here and then validate the count returned?
There was a problem hiding this comment.
Have you addressed this comment?
… and improve session handling
ashutosh-bapat
left a comment
ashutosh-bapat
left a comment
There was a problem hiding this comment.
When you address a comment, please describe the resolution as a response to the comment. It seems there are some review comments still remain unaddressed.
| # Wake up buffercache scan | ||
| $node->safe_psql('postgres', "SELECT injection_points_wakeup('$buffercache_injection_point')"); | ||
|
|
||
| $buffercache_session->query(q(\echo 'done')); |
There was a problem hiding this comment.
Have you addressed this comment?
| #include "utils/rel.h" | ||
| #include "utils/tuplestore.h" | ||
|
|
||
|
|
There was a problem hiding this comment.
Unnecessary line removal. PostgreSQL code usually leaves an extra line between #include section and rest of the code.
| int max_entries; | ||
| char *startptr, | ||
| *endptr; | ||
| int currentNBuffers = pg_atomic_read_u32(&ShmemCtrl->currentNBuffers); |
There was a problem hiding this comment.
Why do we need this change and other changes which use currentNBuffers variable?
There was a problem hiding this comment.
There is a buffer scan loop which should be change to currentNBuffers
| shared_preload_libraries = 'injection_points' | ||
| max_shared_buffers = $shared_buffers_initial | ||
| shared_buffers = $shared_buffers_initial | ||
| max_parallel_workers_per_gather = 0 |
There was a problem hiding this comment.
why do we need max_parallel_workers_per_gather in the test?
| $injection_session->query_safe("SELECT injection_points_attach('$injection_point', 'wait')", verbose => $verbose); | ||
|
|
||
| # Create a new session for resize | ||
| my $resize_session = $node->background_psql('postgres'); |
There was a problem hiding this comment.
Why do we need to do this? Isn't existing resize_session enough?
| note("buffercache query error during $test_name ($operation_type): $query_err") if $query_err; | ||
| $buffercache_session->{stderr} = ''; | ||
|
|
||
| eval { $buffercache_session->quit; }; |
There was a problem hiding this comment.
Why do you need eval{} here?
| eval { $buffercache_session->quit; }; | ||
|
|
||
| # Detach injection point | ||
| eval { |
There was a problem hiding this comment.
Why do you need eval[] here?
| verbose => $verbose); | ||
|
|
||
| my ($buffercache_output, $buffercache_err); | ||
| eval { ($buffercache_output, $buffercache_err) = $buffercache_session->query(q(\echo 'done')); }; |
There was a problem hiding this comment.
Why do we need eval{} here?
…r buffer resizing scenarios
…ction point handling during buffer resizing
…pages.c Revert NBuffers -> currentNBuffers changes in pg_buffercache_os_pages_internal, pg_buffercache_summary, and pg_buffercache_usage_counts as requested by reviewer (comments #7, #15, #22). Only injection point additions and the coupled currentNBuffers change in pg_buffercache_evict are retained. Also fix typo 'curent' -> 'current' in TODO comment, restore missing newline at EOF, and improve test file: add scan count validation, use query_safe consistently, clean up session handling and comments.
2 participants
No description provided.