ADBDEV-3685 Error handling for disqkuota worker startup stage by bimboterminator1 · Pull Request #20 · arenadata/diskquota

bimboterminator1 · 2023-06-13T20:04:38Z

During diskquota worker's first run the initial set of active tables with their sizes is being loaded from diskquota.table_size table in order to warm up diskquota rejectmap and other shared memory objects. If an error occurs during this initialization process, the error will be ignored in PG_CATCH() block. Such ignorance can be potentially harmful and can lead to undesired behaviour of the whole extension. For example, if an error ocurs during initialization, local_active_table_stat_map will not be filled properly. And at the next loop iteration, tables, that are not in acitive table list will be marked as irrelevant and to be deleted both from table_size_map and table_size table in flush_to_table_size function. This situation produces extra perfomance load, which is not guaranteed to be safe.

This commit proposes the handling of the initialization errors, which occur during worker's first run. In the DiskquotaDBEntry structure the bool variable corrupted is added in order to indicate, that the worker wasn't able to initialize itself on given database. And DiskquotaDBEntry also is now passed to refresh_disk_quota_model function from worker main loop, because one need to change the state of dbEntry. The state is changed when the refresh_disk_quota_usage function catches an error, which occured during the initialization step, in PG_CATCH() block. And after the error is catched, the corrupted flag is set in given dbEntry, and then the error is rethrown. This leads to worker process termination. The launcher will not be able to start it again, because added flag is set in the database structure, and this flag is being checked inside the disk_quota_launcher_main function. The flag can be reseted by calling resetBackgroundWorkerCorruption function, which is currently called in SIGHUP handler.

During diskquota worker's first run the initial set of active tables with their sizes is being loaded from diskquota.table_size table in order to warm up diskquota rejectmap and other shared memory objects. If an error occurs during this initialization process, the error will be ignored in PG_CATCH() block. Because of that local_active_table_stat_map will not be filled properly. And at the next loop iteration tables, that are not in acitive table list will be marked as irrelevant and to be deleted both from table_size_map and table_size table in flush_to_table_size function. In case when the inital set of active tables is huge (thousands of tables), this error ignorance could lead to the formation of a too long delete statement, which the SPI executor won't be able to process due to memory limits. And this case can lead to worker's segmentation fault or other errorneous behaviour of whole extension. This commit proposes the handling of the initialization errors, which occur during worker's first run. In the DiskquotaDBEntry structure the bool variable "corrupted" is added in order to indicate, that the worker wasn't able to initialize itself on given database. And DiskquotaDBEntry also is now passed to refresh_disk_quota_model function from worker main loop, because one need to change the state of dbEntry. The state is changed when the refresh_disk_quota_usage function catches an error, which occured during the initialization step, in PG_CATCH() block. And after the error is catched, the "corrupted" flag is set in given dbEntry, and then the error is rethrown. This leads to worker process termination. The launcher will not be able to start it again, because added flag is set in the database structure, and this flag is being checked inside the disk_quota_launcher_main function. The flag can be reseted by calling resetBackgroundWorkerCorruption function, which is currently called in SIGHUP handler.

red1452 · 2023-06-19T17:17:07Z

Could you get patch from PR and do upgrade test with your changes?

bimboterminator1 · 2023-06-19T20:04:13Z

Could you get patch from PR and do upgrade test with your changes?

Have run the upgrade tests in docker with my changes and Eugene's changes. All test passed.

src/quotamodel.c

RekGRpth · 2023-06-27T04:19:15Z

In case when the inital set of active tables is huge (thousands of tables), this error ignorance could lead to the formation of a too long delete statement, which the SPI executor won't be able to process due to memory limits.

I think that in the latest version this is no longer true and another rationale for this patch should be found.

src/diskquota.c

src/diskquota.h

src/diskquota.c

tests/isolation2/sql/test_worker_init_failure.sql

bimboterminator1 · 2023-06-27T12:06:06Z

I think that in the latest version this is no longer true and another rationale for this patch should be found.

I have rewritten PR description.

…#20)" to reaaply above c2686c9 This reverts commit 3b06e37.

During diskquota worker's first run the initial set of active tables with their sizes is being loaded from diskquota.table_size table in order to warm up diskquota rejectmap and other shared memory objects. If an error occurs during this initialization process, the error will be ignored in PG_CATCH() block. Because of that local_active_table_stat_map will not be filled properly. And at the next loop iteration tables, that are not in acitive table list will be marked as irrelevant and to be deleted both from table_size_map and table_size table in flush_to_table_size function. In case when the inital set of active tables is huge (thousands of tables), this error ignorance could lead to the formation of a too long delete statement, which the SPI executor won't be able to process due to memory limits. And this case can lead to worker's segmentation fault or other errorneous behaviour of whole extension. This commit proposes the handling of the initialization errors, which occur during worker's first run. In the DiskquotaDBEntry structure the bool variable "corrupted" is added in order to indicate, that the worker wasn't able to initialize itself on given database. And DiskquotaDBEntry also is now passed to refresh_disk_quota_model function from worker main loop, because one need to change the state of dbEntry. The state is changed when the refresh_disk_quota_usage function catches an error, which occured during the initialization step, in PG_CATCH() block. And after the error is catched, the "corrupted" flag is set in given dbEntry, and then the error is rethrown. This leads to worker process termination. The launcher will not be able to start it again, because added flag is set in the database structure, and this flag is being checked inside the disk_quota_launcher_main function. The flag can be reseted by calling resetBackgroundWorkerCorruption function, which is currently called in SIGHUP handler. Cherry-picked-from: 3b06e37 to reapply above c2686c9

…#20)" This reverts commit 3b06e37.

bimboterminator1 force-pushed the ADBDEV-3685 branch 2 times, most recently from 4451314 to 2bb759b Compare June 16, 2023 11:55

bimboterminator1 changed the title ~~ADBDEV-3685 Bad worker initialization handling~~ ADBDEV-3685 Error handling for disqkuota worker startup stage Jun 16, 2023

bimboterminator1 force-pushed the ADBDEV-3685 branch 2 times, most recently from b5bf85b to 5748cba Compare June 16, 2023 14:44

bimboterminator1 force-pushed the ADBDEV-3685 branch from 5748cba to 215f525 Compare June 16, 2023 15:49

bimboterminator1 marked this pull request as ready for review June 16, 2023 16:46

bimboterminator1 requested a review from a team June 19, 2023 06:18

Stolb27 requested review from RekGRpth and red1452 June 19, 2023 08:57

red1452 reviewed Jun 20, 2023

View reviewed changes

src/quotamodel.c Show resolved Hide resolved

Add lost empty line

82a1d66

red1452 previously approved these changes Jun 20, 2023

View reviewed changes

Removed hardcoded wait time

480b78b

bimboterminator1 dismissed red1452’s stale review via 480b78b June 20, 2023 07:45

red1452 previously approved these changes Jun 23, 2023

View reviewed changes