[#31] Extend pgmoneta_ext_get_files to return sub-directories and metadata

[ashu3103]

Resolve: #31

@jesperpedersen @Jubilee101 PTAL

[ashu3103]

@jesperpedersen The CI is failing due to Pandoc dependency

[jesperpedersen]

@ashu3103 Try and rebase

[ashu3103]

@ashu3103 Try and rebase

@jesperpedersen @Jubilee101 PTAL

[ashu3103]

Hi @jesperpedersen @Jubilee101,

I think this enhancement is not required because what I am trying to achieve is already done in pgmoneta_receive_extra_files, we can directly use that API, its just that we won't be providing any info to it.

Thoughts?

[jesperpedersen]

@ashu3103 Yes, you can see if you can use that function

[ashu3103]

@ashu3103 Yes, you can see if you can use that function

The above trick is working but there is only one issue, the implementation of pgmoneta_receive_extra_files to get file data is giving stack overflow errors

AddressSanitizer:DEADLYSIGNAL
=================================================================
==174001==ERROR: AddressSanitizer: stack-overflow on address 0x7ffd25eb41b8 (pc 0x7bdd992745f9 bp 0x7ffd2a009b90 sp 0x7ffd25eb41c0 T0)
    #0 0x7bdd992745f9 in pgmoneta_log_mem /home/ashu3103/Desktop/pgmoneta/src/libpgmoneta/logging.c:329:10
    #1 0x7bdd9929b089 in pgmoneta_query_execute /home/ashu3103/Desktop/pgmoneta/src/libpgmoneta/message.c:1103:10
    #2 0x7bdd9922b106 in query_execute /home/ashu3103/Desktop/pgmoneta/src/libpgmoneta/extension.c:108:8
    #3 0x7bdd9922b4d4 in pgmoneta_ext_get_file /home/ashu3103/Desktop/pgmoneta/src/libpgmoneta/extension.c:72:11
    #4 0x7bdd992a8a80 in pgmoneta_receive_extra_files /home/ashu3103/Desktop/pgmoneta/src/libpgmoneta/message.c:2427:13
    #5 0x7bdd9941934e in create_standard_directories /home/ashu3103/Desktop/pgmoneta/src/libpgmoneta/wf_backup_incremental.c:627:17
    #6 0x7bdd994165d3 in incr_backup_execute /home/ashu3103/Desktop/pgmoneta/src/libpgmoneta/wf_backup_incremental.c:311:5
    #7 0x7bdd9945e8b9 in pgmoneta_workflow_execute /home/ashu3103/Desktop/pgmoneta/src/libpgmoneta/workflow.c:321:11
    #8 0x7bdd991bdd4a in pgmoneta_backup /home/ashu3103/Desktop/pgmoneta/src/libpgmoneta/backup.c:217:8
    #9 0x633684c3a65c in accept_mgt_cb /home/ashu3103/Desktop/pgmoneta/src/main.c:999:16
    #10 0x7bdd99f2b82a in ev_invoke_pending /build/libev-ntx4dr/libev-4.33/ev.c:3770:11
    #11 0x7bdd99f2f3b0 in ev_run /build/libev-ntx4dr/libev-4.33/ev.c:4190:7
    #12 0x7bdd99f2f3b0 in ev_run /build/libev-ntx4dr/libev-4.33/ev.c:4021:1
    #13 0x633684c38924 in ev_loop /usr/include/ev.h:841:50
    #14 0x633684c2f94b in main /home/ashu3103/Desktop/pgmoneta/src/main.c:814:7
    #15 0x7bdd98c2a1c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #16 0x7bdd98c2a28a in __libc_start_main csu/../csu/libc-start.c:360:3
    #17 0x633684b4ebe4 in _start (/home/ashu3103/Desktop/pgmoneta/build/src/pgmoneta+0x44be4) (BuildId: 6de045a4b5069325f53b7e0f04253b3121555e06)

SUMMARY: AddressSanitizer: stack-overflow /home/ashu3103/Desktop/pgmoneta/src/libpgmoneta/logging.c:329:10 in pgmoneta_log_mem

@jesperpedersen @Jubilee101, can you take a look at it since I'm busy with some other issue.

[Jubilee101]

@ashu3103 I hope pgmoneta/pgmoneta#796 helps.

[ashu3103]

@ashu3103 I hope pgmoneta/pgmoneta#796 helps.

Yeah, it is giving the desired results, thanks!

[jesperpedersen]

@ashu3103 Have you tested this with a user that doesn't have PRIVILEGE_SUPERUSER | PRIVILEGE_PG_READ_SERVER_FILES ?

[Jubilee101]

Getting the following warning

/home/azureuser/pgmoneta_ext/src/pgmoneta_ext/lib.c: In function ‘pgmoneta_ext_get_files’:
/home/azureuser/pgmoneta_ext/src/pgmoneta_ext/lib.c:450:16: warning: array subscript 2 is above array bounds of ‘Datum[2]’ {aka ‘long unsigned int[2]’} [-Warray-bounds=]
  450 |          values[2] = Int64GetDatum(p_info->size);
      |          ~~~~~~^~~
/home/azureuser/pgmoneta_ext/src/pgmoneta_ext/lib.c:443:16: note: while referencing ‘values’
  443 |          Datum values[2];

[ashu3103]

Getting the following warning

/home/azureuser/pgmoneta_ext/src/pgmoneta_ext/lib.c: In function ‘pgmoneta_ext_get_files’:
/home/azureuser/pgmoneta_ext/src/pgmoneta_ext/lib.c:450:16: warning: array subscript 2 is above array bounds of ‘Datum[2]’ {aka ‘long unsigned int[2]’} [-Warray-bounds=]
  450 |          values[2] = Int64GetDatum(p_info->size);
      |          ~~~~~~^~~
/home/azureuser/pgmoneta_ext/src/pgmoneta_ext/lib.c:443:16: note: while referencing ‘values’
  443 |          Datum values[2];

Yeah, this is a bug, don't know why it got through while testing. I'll fix it.

Another important thing to add here is - this enhancement was necessary when we were assuming that checksum has to be calculated at server end, keeping that in mind our workflow for incremental backup now doesn't have any intentions to request checksum from the sever (since we are calculating it at pgmoneta's end) as discussed in discussion forum.

The issue post merging this PR is that the things at pgmoneta side will start to break, as the parsing logic there doesn't expect output in this format, so we may have to change that logic there, which is not a very big deal but I think we are tight on deadline and so we can work or discuss on it later.

[jesperpedersen]

@ashu3103 Do we still need this ?

[ashu3103]

@ashu3103 Do we still need this ?

Yes, it seems we might have to rely on the extension side for directories and sub-directories along with files.
Refer this

I'll prepare this patch and "PTAL" you.

[ashu3103]

@jesperpedersen @Jubilee101, PTAL

I have tested it with a user (repl) that doesn't have PRIVILEGE_SUPERUSER | PRIVILEGE_PG_READ_SERVER_FILES and a user that does have PRIVILEGE_PG_READ_SERVER_FILES for PostgreSQL 14.

For a query:

SELECT pgmoneta_ext_get_files('pg_logical');

Result:

         pgmoneta_ext_get_files         
----------------------------------------
 (pg_logical/replorigin_checkpoint,f,8)
 (pg_logical/mappings,t,4096)
 (pg_logical/snapshots,t,4096)
(3 rows)

[jesperpedersen]

Merged.

Thanks for your contribution !