-
Notifications
You must be signed in to change notification settings - Fork 89
Small fixes #546
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Small fixes #546
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я думаю тогда надо дочистить и упоминание условия в config_get_opt() (потому что в случае агента эта функция вроде и не вызывается).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/utils/file.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А это условие разве остаётся корректным после введения битовых флагов в arg?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
! Спасибо!
src/archive.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А что с этим комментарием? Он актуален?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Хороший вопрос: могут ли у нас под ногами удалить файл?
Думаю, пока можно "забить" и убрать комментарий.
В целом, даже если мы хотим передавать missing_ok=true , нужно различать "не существовавший" файл и файл с crc==0. Так что, придётся переделывать API.
По поводу введения set_forkname:
Первое, за что зацепился глаз -- это то, что функция глобальная, а мне кажется, что не надо выносить это за пределы src/dir.c.
Предлагаю рассматривать pgFileInit() как некоторый "конструктор" "объектов" pgFile и внести логику/вызов заполнения forkName внутрь этого "конструктора".
Тут остаётся вопрос с обработкой external_dir_num, вижу два варианта:
или заполнять поле forkName в любом случае (так как оно зависит от имени), а обработку external_dir_num оставить на внешнюю логику (которая пользуется pgFile)
или вообще сделать два "конструктора" pgFile, один для создания pgFile из файловой системы (из dir_check_file()), второй из бекапа (get_backup_filelist()).
В любом случае в функции get_backup_filelist() смесь логики чтения файла и заполнения внутренних полей pgFile мне кажется очень некрасивой, хотелось бы побольше убрать внутрь src/dir.c.
Но это моё имхо, что думаешь?
Рефакторинг инициализации pgFile выделен в отдельную задачу.
test_basic_ptrack_truncate_replica - локально 20 минут гонял (~25 раз), ни разу не упал
c68c788 to
51a141c
Compare
No description provided.