Resolve issues #5 and #1: reduce number of collisions in the ptrack map#6
Resolve issues #5 and #1: reduce number of collisions in the ptrack map#6
Conversation
…slots. Previously we thought that 1 MB can track changes page-to-page in the 1 GB of data files. However, recently it became evident that our ptrack map or basic hash table behaves more like a Bloom filter with a number of hash functions k = 1. See more here: https://en.wikipedia.org/wiki/Bloom_filter#Probability_of_false_positives. Such filter has naturally more collisions. By storing update_lsn of each block in the additional slot we perform as a Bloom filter with k = 2, which significatly reduces collision rate.
4c32e9e to
38aa439
Compare
Also bump extversion to 2.2
ptrack.c
Outdated
| update_lsn1 = pg_atomic_read_u64(&ptrack_map->entries[slot1]); | ||
| update_lsn2 = pg_atomic_read_u64(&ptrack_map->entries[slot2]); |
There was a problem hiding this comment.
It is better to fetch and check slot1 first, and only if check passed then fetch and check slot2.
This way you will save TLB and cache misses for slot2 for most of page items.
Note that compiler could not optimize/reorder atomic instructions.
There was a problem hiding this comment.
OK, I hope that I did it
Probe the second slot only if the first one succeded.
ptrack--2.1--2.2.sql
Outdated
| FROM | ||
| (SELECT count(path) AS changed_files, | ||
| sum( | ||
| length(replace(right((pagemap)::text, -1)::varbit::text, '0', '')) |
There was a problem hiding this comment.
Если таблицы 8TB, то вот эта строчка потребует выделение 1GB памяти для преобразования ::varbit::text.
Соответственно, таблица 16TB потребует уже 2GB памяти, и постгресс просто сам не позволит этого сделать.
There was a problem hiding this comment.
Это очень грустно, что varbit не имеет функции countbits.
There was a problem hiding this comment.
В любом случае, для ptrack_get_change_stat и ptrack_get_change_file_stat кажется нужно создать ptrack_get_pagecount (ну или другое название).
Или даже просто реализовать ptrack_get_change_file_stat полностью в сишке.
There was a problem hiding this comment.
Таблицы же разбиты на сегменты по 1 ГБ дефолтно, а ptrack_get_pagemapset() выдаёт изначально битмапы per file/segment, то есть потребуется максимум в 1000 раз меньше памяти на каждое преобразование. Разве нет?
There was a problem hiding this comment.
А ок. Я ещё не посмотрел ptrack_get_pagemapset() .
There was a problem hiding this comment.
Слушай, но я бы всё равно поменял бы ptrack_get_pagemapset, добавив поле count в вывод.
pg_probackup при этом не поломается, т.к. он указывает поля, которые хочет.
|
|
||
| /* Delete and try again */ | ||
| durable_unlink(ptrack_path, LOG); | ||
| is_new_map = true; |
There was a problem hiding this comment.
Не могу найти, где делается unmap в этом случае?
При этом сразу после метки ptrack_map_reinit делается durable_unlink(ptrack_mmap_path).
В итоге, этот файл повисает невидимкой в файловой системе, и в адрессном пространстве процесса повисает его mmap.
Наверное есть смысл позвать здесь ptrackCleanFilesAndMap ?
There was a problem hiding this comment.
Да, похоже на то. Я сомневался в этом месте, но потом забыл и не разобрался до конца
|
Everything seems to be working, so I'm merging this one. If the internal QA finds out anything, we will fix it in |
Resolve #5
Resolve #1