[PGPRO-5691] move mmapped ptrack map into shared postgres memory #19
Conversation
|
Вопрос ревьюирам: как думаете, нужно ли удалять строку |
| /* | ||
| * XXX: for some reason assign_ptrack_map_size is called twice during the | ||
| * postmaster boot! First, it is always called with bootValue, so we use | ||
| * -1 as default value and no-op here. Next, it is called with the actual | ||
| * value from config. That way, we use 0 as an option for user to turn | ||
| * off ptrack and clean up all files. | ||
| */ | ||
| if (newval == -1) | ||
| return; |
There was a problem hiding this comment.
Is it not a problem anymore? I see you use 0 as a boot value and clean up files when ptrack_map_size == 0
There was a problem hiding this comment.
We now do nothing (with files and memory) inside assign_ptrack_map_size() and use the value of ptrack_map_size after the re-assignment has passed.
Well, at least that's how I understand it.
Yeah, you don't write this file, so I don't think that we need this check in tests anymore |
* fix tap test (remove checking of ptrack.map.mmap file) * cosmetic change (minimize diff with master)
engine.c
Outdated
| elog(ERROR, "ptrack read map: unexpected end of file while reading map file \"%s\", expected to read %zu, but read only %zu bytes", | ||
| ptrack_path, PtrackActualSize, readed); | ||
| } | ||
| else if (last_readed < 0) |
There was a problem hiding this comment.
Надо на EINTR проверить. И...
Посмотрел на искось код постгресса, практически везде просто делают continue. Т.е. ни какой специальной реакции типа CHECK_FOR_INTERRUPTS обычно нет.
There was a problem hiding this comment.
Добавил проверку EINTR. CHECK_FOR_INTERRUPTS не добавлял.
Заодно убрал дублирующее readed += last_readed, которое каким-то чудом проскочило через предыдущие запуски тестов.
engine.c
Outdated
|
|
||
| #ifdef WIN32 | ||
| /* Check ptrack version inside old ptrack map */ | ||
| if (ptrack_map->version_num < PTRACK_COMPATIBLE_VERSION_NUM) |
There was a problem hiding this comment.
Здесь, думаю, всё-таки нужно != : вдруг следующая версия будет с багом, и кто-нибудь решит сдаунгрейдиться.
engine.c
Outdated
| * Unconditionally update version | ||
| * This is usefull if we have read early compatible version of map | ||
| */ | ||
| ptrack_map->version_num = PTRACK_VERSION_NUM; |
There was a problem hiding this comment.
А почему не PTRACK_COMPATIBLE_VERSION_NUM ?
Соответственно, это нужно только под if (is_new_map)
| if (ptrack_map_size != 0) | ||
| RequestAddinShmemSpace(PtrackActualSize); | ||
| else | ||
| ptrackCleanFiles(); |
There was a problem hiding this comment.
А к этому моменту уже был позван assign_ptrack_map_size ?
#471) * [PGPRO-5691] ptrack-2.3: move mmapped ptrack map into shared postgres memory In ptrack-2.3 ptrack.map.mmap will be removed and 'incorrect checksum' error will not be fatal (postgrespro/ptrack#19) * added test_corrupt_ptrack_map test compatibility with both version 2.2 and version 2.3 of ptrack
Codecov Report
@@ Coverage Diff @@
## master #19 +/- ##
==========================================
+ Coverage 88.55% 89.07% +0.52%
==========================================
Files 2 2
Lines 428 421 -7
==========================================
- Hits 379 375 -4
+ Misses 49 46 -3
Continue to review full report at Codecov.
|
No description provided.