Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit8e03b8d

Browse files
committed
Review answer#1
1 parent271cf16 commit8e03b8d

File tree

3 files changed

+26
-18
lines changed

3 files changed

+26
-18
lines changed

‎src/catchup.c

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ catchup_collect_info(PGNodeInfo*source_node_info, const char *source_pgdata, co
7272
/* Get WAL segments size and system ID of source PG instance */
7373
instance_config.xlog_seg_size=get_xlog_seg_size(source_pgdata);
7474
instance_config.system_identifier=get_system_identifier(source_pgdata);
75+
#ifPG_VERSION_NUM<90600
76+
instance_config.pgdata=source_pgdata;
77+
#endif
7578
current.start_time=time(NULL);
7679

7780
StrNCpy(current.program_version,PROGRAM_VERSION,sizeof(current.program_version));
@@ -113,7 +116,10 @@ catchup_collect_info(PGNodeInfo*source_node_info, const char *source_pgdata, co
113116
returnsource_conn;
114117
}
115118

116-
//REVIEW Please add a comment to this function.
119+
/*
120+
* Check that catchup can be performed on source and dest
121+
* this function is for checks, that can be performed without modification of data on disk
122+
*/
117123
staticvoid
118124
catchup_preflight_checks(PGNodeInfo*source_node_info,PGconn*source_conn,
119125
constchar*source_pgdata,constchar*dest_pgdata,booldest_pgdata_is_empty)
@@ -164,16 +170,13 @@ do_catchup_instance(const char *source_pgdata, const char *dest_pgdata, PGconn *
164170
RedoParamsdest_redo= {0,InvalidXLogRecPtr,0 };
165171
pgFile*source_pg_control_file=NULL;
166172

167-
//REVIEW please adjust this comment.
168-
/* arrays with meta info for multi threaded backup */
173+
/* arrays with meta info for multi threaded catchup */
169174
pthread_t*threads;
170175
catchup_thread_runner_arg*threads_args;
171176
boolcatchup_isok= true;
172177

173178
parray*source_filelist=NULL;
174179
parray*dest_filelist=NULL;
175-
//REVIEW We don't handle external_dirs in catchup, do we? Let's clean this up.
176-
parray*external_dirs=NULL;
177180

178181
//REVIEW FIXME Let's fix it before release. It can cause some obscure bugs.
179182
/* TODO: in case of timeline mistmatch, check that source PG timeline descending from dest PG timeline */
@@ -199,8 +202,6 @@ do_catchup_instance(const char *source_pgdata, const char *dest_pgdata, PGconn *
199202
strncat(label," with pg_probackup",lengthof(label)-
200203
strlen(" with pg_probackup"));
201204

202-
//REVIEW FIXME Let' do that.
203-
204205
/* Call pg_start_backup function in PostgreSQL connect */
205206
pg_start_backup(label,smooth_checkpoint,&current,source_node_info,source_conn);
206207
elog(LOG,"pg_start_backup START LSN %X/%X", (uint32) (current.start_lsn >>32), (uint32) (current.start_lsn));
@@ -231,8 +232,8 @@ do_catchup_instance(const char *source_pgdata, const char *dest_pgdata, PGconn *
231232

232233
// new ptrack is more robust and checks Start LSN
233234
if (ptrack_lsn>dest_redo.lsn||ptrack_lsn==InvalidXLogRecPtr)
234-
elog(ERROR,"LSN from ptrack_control %X/%X is greater than checkpoint LSN %X/%X.\n"
235-
"Create new full backup before an incremental one.",
235+
elog(ERROR,"LSN from ptrack_controlin source%X/%X is greater than checkpoint LSNin destination %X/%X.\n"
236+
"You can perform only FULL catchup.",
236237
(uint32) (ptrack_lsn >>32), (uint32) (ptrack_lsn),
237238
(uint32) (dest_redo.lsn >>32),
238239
(uint32) (dest_redo.lsn));
@@ -255,8 +256,6 @@ do_catchup_instance(const char *source_pgdata, const char *dest_pgdata, PGconn *
255256
current.start_lsn,current.tli);
256257
}
257258

258-
//REVIEW please adjust the comment.
259-
/* initialize backup list */
260259
source_filelist=parray_new();
261260

262261
/* list files with the logical path. omit $PGDATA */
@@ -471,14 +470,14 @@ do_catchup_instance(const char *source_pgdata, const char *dest_pgdata, PGconn *
471470
//REVIEW Hmm. Why do we need this at all?
472471
//I'd expect that we init pgfile with unset lock...
473472
//Not related to this patch, though.
473+
//REVIEW_ANSWER initialization in the pgFileInit function was proposed but was not accepted (see 2c8b7e9)
474474
/* clear file locks */
475475
pfilearray_clear_locks(source_filelist);
476476

477477
/* Sort by size for load balancing */
478478
parray_qsort(source_filelist,pgFileCompareSize);
479479

480-
//REVIEW. This comment looks a bit misleading, since all theads share same filelist.
481-
/* init thread args with own file lists */
480+
/* init thread args */
482481
threads= (pthread_t*)palloc(sizeof(pthread_t)*num_threads);
483482
threads_args= (catchup_thread_runner_arg*)palloc(sizeof(catchup_thread_runner_arg)*num_threads);
484483

@@ -539,6 +538,7 @@ do_catchup_instance(const char *source_pgdata, const char *dest_pgdata, PGconn *
539538
pretty_time);
540539

541540
//REVIEW The comment looks unrelated to the function. Do I miss something?
541+
//REVIEW_ANSWER because it is a part of pg_stop_backup() calling
542542
/* Notify end of backup */
543543
pg_silent_client_messages(source_conn);
544544

@@ -681,12 +681,9 @@ do_catchup_instance(const char *source_pgdata, const char *dest_pgdata, PGconn *
681681
parray_walk(dest_filelist,pgFileFree);
682682
parray_free(dest_filelist);
683683
}
684-
685684
parray_walk(source_filelist,pgFileFree);
686685
parray_free(source_filelist);
687686
pgFileFree(source_pg_control_file);
688-
//REVIEW Huh?
689-
// где закрывается conn?
690687
}
691688

692689
/*

‎src/utils/file.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2001,7 +2001,7 @@ fio_copy_pages(const char *to_fullpath, const char *from_fullpath, pgFile *file,
20012001

20022002
COMP_FILE_CRC32(true,file->crc,buf,hdr.size);
20032003

2004-
elog(INFO,"Copy block %u with size %u of %s",blknum,hdr.size-sizeof(BackupPageHeader),to_fullpath);
2004+
elog(INFO,"Copy block %u with size %lu of %s",blknum,hdr.size-sizeof(BackupPageHeader),to_fullpath);
20052005
if (fio_fseek(out,blknum*BLCKSZ)<0)
20062006
{
20072007
elog(ERROR,"Cannot seek block %u of \"%s\": %s",

‎tests/catchup.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ def test_multithread_local_transfer(self):
1414
"""
1515
fname=self.id().split('.')[3]
1616

17-
source_pg=self.make_simple_node(base_dir=os.path.join(module_name,fname,'src'))
17+
source_pg=self.make_simple_node(
18+
base_dir=os.path.join(module_name,fname,'src'),
19+
set_replication=True)
1820
source_pg.slow_start()
1921
source_pg.safe_psql(
2022
"postgres",
@@ -140,6 +142,9 @@ def test_remote_ptrack_catchup(self):
140142
generate some load on master, insert some test data on master,
141143
catchup copy, start and select test data
142144
"""
145+
ifnotself.ptrack:
146+
returnunittest.skip('Skipped because ptrack support is disabled')
147+
143148
fname=self.id().split('.')[3]
144149

145150
# prepare master
@@ -263,6 +268,9 @@ def test_remote_delta_catchup(self):
263268
deftest_table_drop(self):
264269
"""
265270
"""
271+
ifnotself.ptrack:
272+
returnunittest.skip('Skipped because ptrack support is disabled')
273+
266274
fname=self.id().split('.')[3]
267275

268276
source_pg=self.make_simple_node(
@@ -317,6 +325,9 @@ def test_table_drop(self):
317325
deftest_tablefile_truncation(self):
318326
"""
319327
"""
328+
ifnotself.ptrack:
329+
returnunittest.skip('Skipped because ptrack support is disabled')
330+
320331
fname=self.id().split('.')[3]
321332

322333
source_pg=self.make_simple_node(

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp