ADBDEV-4015 2.2.2 sync #23
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Note: `TextDatumGetCString()` returns a null-terminated CString.
```
[ 12%] Building C object CMakeFiles/diskquota.dir/src/diskquota.c.o
In file included from /home/v/workspace/diskquota/src/diskquota.h:16,
from /home/v/workspace/diskquota/src/diskquota.c:18:
/home/v/workspace/diskquota/src/diskquota.c: In function ‘diskquota_status_schema_version’:
/home/v/.local/gpdb7/include/postgresql/server/c.h:957:25: warning: ‘strncpy’ specified bound 64 equals destinatio
n size [-Wstringop-truncation]
957 | strncpy(_dst, (src), _len); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
/home/v/workspace/diskquota/src/diskquota.c:1548:9: note: in expansion of macro ‘StrNCpy’
1548 | StrNCpy(version, vv, sizeof(version));
| ^~~~~~~
```
Co-authored-by: Hao Zhang <[email protected]>
Fix bug produced by #339 Co-authored-by: Xing Guo [email protected]
fix pipeline resource
# Problem Recording active tables and permission checking on segments rely on `monitored_dbid_cache`. But after mirror switching, the data in shared memory is lost, and the above behaviors will be blocked. # Solution Segments update their `monitored_dbid_cache` after receiving pull_active_tables_oid requests every naptime.
To achieve upgrading directly from 2.0 to 2.2, we should do the following things: - cherry-pick this PR in diskquota-2.0, diskquota-2.1 and diskquota-2.2. - set the shared_preload_libraries as `diskquota-2.2`: `gpconfig -c shared_preload_libraries -v 'diskquota-2.2'` - restart cluster: `gpstop -ar` - execute the following SQLs: ``` ALTER extension diskquota update to '2.2'; ```
Get the latest package version for release resource.
- Because of 97f1f9b, the sql file directory has been changed. The sql version check wouldn't work since it cannot find the sql files anymore. Change it to the correct ddl directory. - 'exec_program' is deprecated, use 'execute_process' instead. - 'git tag | sort' returns the latest version among all branches, but not the closest tag to the current commit. Use 'git describe --tags' instead. So the upgrade version check will work for the 2.1.x patch release.
After creating extension, the `diskquota.state` is always `unready` due to the change greenplum-db/gpdb#15239. It makes the test timeout. We disable the job of gp7 in release/pr/merge pipeline until the problem is fixed.
- By #340, diskquota should be able to upgraded directly from any previous version. A script is added to test this. - Modify the cmakefile so before installing/packaing, only previous so files will copied. This would help us to make patch release for 2.0/2.1. --------- Co-authored-by: Zhang Hao <[email protected]>
Co-authored-by: Chen Mulong [email protected]
- bump version to 2.2.2 - Modify the check procedure of alter extension.
Fix from #346
git-subtree-dir: concourse/lib git-subtree-split: d51adf5d59e4e5caefb678b29c4335881040c7d2
Migrage changes pivotal/pgaudit#13 here
|
@RekGRpth, as far I understand, your patch 472eb06 still actual despite upstream modifications from 05da9d4 and 86ff586 because diskquota still loads shared libraries of intermediate versions during executing CREATE FUNCTION in the extension script. They solved the problem by ignoring initialization logic. But they expect that user preserve previous versions of the library. |
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
|
@red1452, these commits 05da9d4 and 86ff586 seem to solve the problem #21 (comment). |
RekGRpth
previously approved these changes
Jul 26, 2023
red1452
reviewed
Jul 26, 2023
| else | ||
| match = false; | ||
| pfree(query); | ||
| return match; |
There was a problem hiding this comment.
This function is incorrect if it is necessary that ActivePortal->sourceText must contain word update. This word is not checked at this function.
There was a problem hiding this comment.
This word is not checked at this function.
Why?
There was a problem hiding this comment.
After save pos at line 192 there is check pos is not null at line 195. This is check that query contains diskquota. If it is true, match is not change to false. Next saving pointer of update to pos does not affect to variable match. And true will be returned even if strstr(pos, "update"); returns NULL.
There was a problem hiding this comment.
Can you explain step by step?
182: query = "alter extension diskquota update"
184: match = true
186: pos = "alter extension diskquota update"
188: pos = "extension diskquota update"
192: pos = "diskquota update"
196: pos = "update"
200: match = true
There was a problem hiding this comment.
If you type another word instead of update you will get match = true but expect match = false.
There was a problem hiding this comment.
Exactly! If last word is not update then
196: pos = NULL
They missed another check
if (pos)
pos = strstr(pos, "update");
else
match = false;
if (!pos)
match = false;existing upgrade and alter tests are useless for ADB: - previous releases incompatible with current due PG-module version check - our upgrade process exclude existing of previous versions during upgrade
|
Currently, diskquota 2.2.2 is affected by greenplum-db#360. Delayed. |
Stolb27
reviewed
Jul 27, 2023
| { | ||
| if (ActivePortal == NULL) return false; | ||
| /* QD: When the sourceTag is T_AlterExtensionStmt, then return true */ | ||
| if (ActivePortal->sourceTag == T_AlterExtensionStmt) return true; |
There was a problem hiding this comment.
But we don't return false on QD otherwise. As a result, we anyway validate sourceText that may contain any garbage.
|
Closed in favor of #26 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.