From 2d51cf40b8765d8140960746552a742c907211eb Mon Sep 17 00:00:00 2001 From: Jimmy Yih Date: Wed, 23 Jan 2019 18:19:25 -0800 Subject: [PATCH] Remove unused XLogAODropSegmentFile function The XLogAODropSegmentFile function was used for implementing invalid page logging for AO. In the past year, there have been significant changes after removing MMXLOG_REMOVE_FILE XLOG record type that make this function no longer needed (currently only being used by a unit test). The most relevant change is that deletion of AO segment files only happen during DROP TABLE (vacuum was changed to truncate the segment file) which makes things easier for AO to reuse the existing invalid page logging implementation. References: https://github.com/greenplum-db/gpdb/commit/175c25e8fb0494933087ff19ef29d7377e021702 https://github.com/greenplum-db/gpdb/commit/8165e1b1627b9b2f8b40c3c55eab9de3d137b70a https://github.com/greenplum-db/gpdb/commit/8838ac983c66af74d0a1422337cf92f08fbe5f2c --- src/backend/access/transam/test/Makefile | 2 - .../access/transam/test/xlogutils_test.c | 53 ------------------- src/backend/access/transam/xlogutils.c | 42 +++------------ src/include/access/xlogutils.h | 1 - 4 files changed, 6 insertions(+), 92 deletions(-) delete mode 100644 src/backend/access/transam/test/xlogutils_test.c diff --git a/src/backend/access/transam/test/Makefile b/src/backend/access/transam/test/Makefile index 043465bfa5..b017bf3bcf 100644 --- a/src/backend/access/transam/test/Makefile +++ b/src/backend/access/transam/test/Makefile @@ -4,8 +4,6 @@ include $(top_builddir)/src/Makefile.global TARGETS=xact distributedlog xlog varsup -TARGETS += xlogutils - include $(top_builddir)/src/backend/mock.mk distributedlog.t: \ diff --git a/src/backend/access/transam/test/xlogutils_test.c b/src/backend/access/transam/test/xlogutils_test.c deleted file mode 100644 index ce56493f41..0000000000 --- a/src/backend/access/transam/test/xlogutils_test.c +++ /dev/null @@ -1,53 +0,0 @@ -#include -#include -#include -#include "cmockery.h" - -#include "../xlogutils.c" - -#include "catalog/pg_tablespace.h" - -/* - * Test that the invalid_page_tab hash table is properly used when invalid AO - * segment files are encountered. - */ -void -test_ao_invalid_segment_file(void **state) -{ - RelFileNode relfilenode; - int32 segmentFileNum; - - /* create mock relfilenode */ - relfilenode.spcNode = DEFAULTTABLESPACE_OID; - relfilenode.dbNode = 12087 /* postgres database */; - relfilenode.relNode = FirstNormalObjectId; - - segmentFileNum = 2; - - /* invalid_page_tab should not be initialized */ - assert_true(invalid_page_tab == NULL); - - /* initialize invalid_page_tab and log an invalid AO segment file */ - XLogAOSegmentFile(relfilenode, segmentFileNum); - assert_false(invalid_page_tab == NULL); - assert_int_equal(hash_get_num_entries(invalid_page_tab), 1); - - /* try again but should not put in same entry */ - XLogAOSegmentFile(relfilenode, segmentFileNum); - assert_int_equal(hash_get_num_entries(invalid_page_tab), 1); - - /* forget the invalid AO segment file */ - XLogAODropSegmentFile(relfilenode, segmentFileNum); - assert_int_equal(hash_get_num_entries(invalid_page_tab), 0); -} - -int -main(int argc, char* argv[]) -{ - cmockery_parse_arguments(argc, argv); - - const UnitTest tests[] = { - unit_test(test_ao_invalid_segment_file) - }; - return run_tests(tests); -} diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c index 549b549360..7f6302fe88 100644 --- a/src/backend/access/transam/xlogutils.c +++ b/src/backend/access/transam/xlogutils.c @@ -203,31 +203,6 @@ forget_invalid_pages_db(Oid dbid) } } -/* Forget an invalid AO/AOCO segment file */ -static void -forget_invalid_segment_file(RelFileNode rnode, uint32 segmentFileNum) -{ - xl_invalid_page_key key; - bool found; - - if (invalid_page_tab == NULL) - return; /* nothing to do */ - - key.node = rnode; - key.forkno = MAIN_FORKNUM; - key.blkno = segmentFileNum; - hash_search(invalid_page_tab, - (void *) &key, - HASH_FIND, &found); - if (!found) - return; - - if (hash_search(invalid_page_tab, - (void *) &key, - HASH_REMOVE, &found) == NULL) - elog(ERROR, "hash table corrupted"); -} - /* Are there any unresolved references to invalid pages? */ bool XLogHaveInvalidPages(void) @@ -411,10 +386,12 @@ XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum, /* * If the AO segment file does not exist, log the relfilenode into the - * invalid_page_table hash table using the segment file number as the - * block number to avoid creating a new hash table. The entry will be - * removed if there is a following MMXLOG_REMOVE_FILE record for the - * relfilenode. + * invalid_page_table hash table using the segment file number as the block + * number to avoid creating a new hash table specifically for AO. The entry + * will be removed if there is a following xlog redo commit prepared record + * for deleting the relfilenode. The segment file number here is only used + * for a debug message since XLogDropRelation logic will remove all + * invalid_page_tab entries that have the same relfilenode and fork number. */ void XLogAOSegmentFile(RelFileNode rnode, uint32 segmentFileNum) @@ -508,13 +485,6 @@ XLogDropRelation(RelFileNode rnode, ForkNumber forknum) forget_invalid_pages(rnode, forknum, 0); } -/* Drop an AO/CO segment file from the invalid_page_tab hash table */ -void -XLogAODropSegmentFile(RelFileNode rnode, uint32 segmentFileNum) -{ - forget_invalid_segment_file(rnode, segmentFileNum); -} - /* * Drop a whole database during XLOG replay * diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h index dc36f2d4c3..ea13c54d47 100644 --- a/src/include/access/xlogutils.h +++ b/src/include/access/xlogutils.h @@ -30,6 +30,5 @@ extern Relation CreateFakeRelcacheEntry(RelFileNode rnode); extern void FreeFakeRelcacheEntry(Relation fakerel); extern void XLogAOSegmentFile(RelFileNode rnode, uint32 segmentFileNum); -extern void XLogAODropSegmentFile(RelFileNode rnode, uint32 segmentFileNum); #endif -- GitLab