[CBRD-23620] It can't be found free pages, even if there are free pages on the temp temp volume
http://jira.cubrid.org/browse/CBRD-23620
fixed a problem which didn't be found free page from temp temp vol in certain cases. And, it has been changed to expand only as much as intended in the case of temp temp volume. The calculation method of disk_Temp_max_sects has been modified to avoid user confusion due to the difference in units between page and sector.
Regarding disk_reserve_from_cache(), you seem to be changing the spec like below: AS-IS: When to reserve sectors, the disk manager prioritizes a permanent type volume. For example, when 5 sectors are requested and there are 3 sectors left in a perm type volume, and 10 sectors left in a temp volume, 3 sectors in the perm volume and 2 sectors in a temp volume are reserved.
TO-BE: In the same case above, only 5 sectors in a temp volume is reserved.
What I understand is right and what you intended? To review the code, please define what the problem was and elaborate on the changed design of sector reservation to handle the problem.
@wrlawodms Thank you for review. It is not intended to change the specification in this section. This is a wrong fixed code. I will revert this code.
@mhoh3963 Can I review this PR after you fix the indent violation?
@hornetmj completed to fix code style.
Codecov Report
Merging #3112 (a09ca0e) into develop (64eba92) will decrease coverage by
11.61%. The diff coverage is55.55%.
:exclamation: Current head a09ca0e differs from pull request most recent head 587d9ea. Consider uploading reports for the commit 587d9ea to get more accurate results
@@ Coverage Diff @@
## develop #3112 +/- ##
============================================
- Coverage 52.31% 40.69% -11.62%
============================================
Files 498 498
Lines 393577 392232 -1345
============================================
- Hits 205898 159637 -46261
- Misses 187679 232595 +44916
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/base/system_parameter.c | 54.36% <ø> (-1.20%) |
:arrow_down: |
| src/storage/disk_manager.c | 44.38% <55.55%> (-0.04%) |
:arrow_down: |
| src/loaddb/load_common.hpp | 0.00% <0.00%> (-100.00%) |
:arrow_down: |
| src/loaddb/load_sa_loader.hpp | 0.00% <0.00%> (-100.00%) |
:arrow_down: |
| src/object/object_printer.hpp | 0.00% <0.00%> (-100.00%) |
:arrow_down: |
| src/base/memory_reference_store.hpp | 0.00% <0.00%> (-94.60%) |
:arrow_down: |
| src/base/memory_private_allocator.hpp | 0.00% <0.00%> (-86.67%) |
:arrow_down: |
| src/query/scan_json_table.cpp | 0.00% <0.00%> (-83.11%) |
:arrow_down: |
| src/loaddb/load_driver.cpp | 0.00% <0.00%> (-81.82%) |
:arrow_down: |
| src/compat/db_json_path.cpp | 0.00% <0.00%> (-78.07%) |
:arrow_down: |
| ... and 214 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 64eba92...587d9ea. Read the comment docs.
@mhoh3963 I executed the 'Repro' in the Jira and I faced the same problem again. Please check it again. And The changed error message still doesn't seem to be correct. The temp_file_max_size_in_pages system parameter is set to 2000. However, the error message shows 4096 pages like the following:
ERROR: The 4096 pages of total temporary space allowed have been exceeded.
@hornetmj When run test.sh, you can see count result after message ("run select.sql again") as following. Do you get error, when run test.sh ?
=== <Result of SELECT Command in Line 9> ===
count(*)
====================== 1000000
And I modified to display the actual extended volume page, not value of temp_file_max_size_in_pages system parameter.
@mhoh3963 I didn't use test.sh. I just do a manual test. About the error message, the temp_file_max_size_in_pages system parameter makes the limitation of the temp file size by page count. The error message tells that our system didn't follow the system parameter setting.
@hornetmj @wrlawodms I modified codes as the following, please start to review. If you need to review at the off-line, let me know.
- fix to run test scenarios in the debug mode by removing assert.
- modify how to find free spaces at temp volumes. AS-IS) * find free spaces at permanent temp volumes. * check that temp volume can extend, if don't find or enough its at permanent temp volumes. TO-BE) * find free spaces at permanent temp volumes. * find free spaces at temporary temp volumes. * check that temp volume can extend, if don't find or enough its at temp volumes (permanent and temporary volumes).
- separate codes to find free spaces on temp and data volume for readability.
- change that temporary temp volume doesn't extend over than disk_Temp_max_sects at disk_extend().
- didn't move a limit checking part of temporary temp volume to disk_extend(), because wait by mutex
- when executing repro with temp_file_max_size_in_pages=2000 , you can face error because I modify to extend temporary volume to 4096 pages not 8192 pages. you need to set system parameter (max_pages_in_temp_file_cache=100) for facing success.
@mhoh3963 The review started late. Sorry for this. We will proceed with the review. (@wrlawodms, @eido5)
I close this PR because there has been no progress for a long time. It would be nice to make another PR after applying feedbacks or changing the way to solve the problem.