cubrid icon indicating copy to clipboard operation
cubrid copied to clipboard

[CBRD-23620] It can't be found free pages, even if there are free pages on the temp temp volume

Open mhoh3963 opened this issue 4 years ago • 9 comments

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.

mhoh3963 avatar Oct 21 '21 03:10 mhoh3963

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 avatar Oct 21 '21 07:10 mhoh3963

@mhoh3963 Can I review this PR after you fix the indent violation?

hornetmj avatar Oct 21 '21 10:10 hornetmj

@hornetmj completed to fix code style.

mhoh3963 avatar Oct 22 '21 04:10 mhoh3963

Codecov Report

Merging #3112 (a09ca0e) into develop (64eba92) will decrease coverage by 11.61%. The diff coverage is 55.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 Impacted file tree graph

@@             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 data Powered by Codecov. Last update 64eba92...587d9ea. Read the comment docs.

codecov-commenter avatar Oct 26 '21 13:10 codecov-commenter

@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 avatar Oct 28 '21 06:10 hornetmj

@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 avatar Oct 28 '21 07:10 mhoh3963

@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 avatar Oct 28 '21 08:10 hornetmj

@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 avatar Nov 18 '21 04:11 mhoh3963

@mhoh3963 The review started late. Sorry for this. We will proceed with the review. (@wrlawodms, @eido5)

hornetmj avatar Dec 10 '21 08:12 hornetmj

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.

hornetmj avatar Nov 07 '22 07:11 hornetmj