Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix file trailer handling in the copy fetcher #5410

Merged
merged 1 commit into from Mar 9, 2023

Conversation

jnidzwetzki
Copy link
Contributor

@jnidzwetzki jnidzwetzki commented Mar 7, 2023

The copy fetcher fetches tuples in batches. When the last element in the batch is the file trailer, the trailer was not handled correctly. The existing logic did not perform a PQgetCopyData in that case. Therefore the state of the fetcher was not set to EOF and the copy operation was not correctly finished at this point.

Fixes: #5323, #5394

@jnidzwetzki jnidzwetzki changed the title Fix handling of file trailer in the copy fetcher Fix file trailer handling in the copy fetcher Mar 7, 2023
@codecov
Copy link

codecov bot commented Mar 7, 2023

Codecov Report

Merging #5410 (96574a7) into main (a854b27) will decrease coverage by 0.02%.
The diff coverage is 89.47%.

@@            Coverage Diff             @@
##             main    #5410      +/-   ##
==========================================
- Coverage   90.70%   90.69%   -0.02%     
==========================================
  Files         227      227              
  Lines       52611    52613       +2     
==========================================
- Hits        47723    47715       -8     
- Misses       4888     4898      +10     
Impacted Files Coverage Δ
tsl/src/remote/copy_fetcher.c 85.57% <89.47%> (+0.34%) ⬆️
src/loader/bgw_message_queue.c 86.36% <0.00%> (-2.85%) ⬇️
src/loader/bgw_launcher.c 89.51% <0.00%> (-2.55%) ⬇️
tsl/src/bgw_policy/job.c 87.54% <0.00%> (-0.05%) ⬇️
src/compat/compat.h 96.61% <0.00%> (+6.13%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jnidzwetzki jnidzwetzki marked this pull request as ready for review March 7, 2023 13:39
@svenklemm
Copy link
Member

Might also fix #5394

Copy link
Member

@akuzm akuzm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Maybe we can get rid of file_trailer_received now?

@jnidzwetzki
Copy link
Contributor Author

@akuzm Looks like we are using it only for Asserts now. Will open a follow-up PR to remove it from release builds.

The copy fetcher fetches tuples in batches. When the last element in the
batch is the file trailer, the trailer was not handled correctly. The
existing logic did not perform a PQgetCopyData in that case. Therefore
the state of the fetcher was not set to EOF and the copy operation was
not correctly finished at this point.

Fixes: timescale#5323
@jnidzwetzki jnidzwetzki enabled auto-merge (rebase) March 9, 2023 13:03
jnidzwetzki added a commit to jnidzwetzki/timescaledb that referenced this pull request Mar 9, 2023
The PR timescale#5410 changes the handling of the file_trailer_received flag. It
is now only used in asserts and not in any other kind of logic. This
patch encapsulates the file_trailer_received with a USE_ASSERT_CHECKING
macro.
@jnidzwetzki jnidzwetzki merged commit 7b8177a into timescale:main Mar 9, 2023
jnidzwetzki added a commit to jnidzwetzki/timescaledb that referenced this pull request Mar 9, 2023
The PR timescale#5410 changes the handling of the file_trailer_received flag. It
is now only used in asserts and not in any other kind of logic. This
patch encapsulates the file_trailer_received in a USE_ASSERT_CHECKING
macro.
akuzm added a commit to akuzm/timescaledb that referenced this pull request Apr 19, 2023
 ## 2.10.2 (2023-04-20)

**Bugfixes**
* timescale#5410 Fix file trailer handling in the COPY fetcher
* timescale#5446 Add checks for malloc failure in libpq calls
* timescale#5233 Out of on_proc_exit slots on guc license change
* timescale#5428 Use consistent snapshots when scanning metadata
* timescale#5499 Do not segfault on large histogram() parameters
* timescale#5470 Ensure superuser perms during copy/move chunk
* timescale#5500 Fix when no FROM clause in continuous aggregate definition
* timescale#5433 Fix join rte in CAggs with joins
* timescale#5556 Fix duplicated entries on timescaledb_experimental.policies view
* timescale#5462 Fix segfault after column drop on compressed table
* timescale#5543 Copy scheduled_jobs list before sorting it
* timescale#5497 Allow named time_bucket arguments in Cagg definition
* timescale#5544 Fix refresh from beginning of Continuous Aggregate with variable time bucket
* timescale#5558 Use regrole for job owner
* timescale#5542 Enable indexscan on uncompressed part of partially compressed chunks

**Thanks**
* @nikolaps for reporting an issue with the COPY fetcher
* @S-imo-n for reporting the issue on Background Worker Scheduler crash
* @geezhu for reporting issue on segfault in historgram()
* @mwahlhuetter for reporting the issue with joins in CAggs
* @mwahlhuetter for reporting issue with duplicated entries on timescaledb_experimental.policies view
* @H25E for reporting error refreshing from beginning of a Continuous Aggregate with variable time bucket
akuzm added a commit that referenced this pull request Apr 19, 2023
 ## 2.10.2 (2023-04-20)

**Bugfixes**
* #5410 Fix file trailer handling in the COPY fetcher
* #5446 Add checks for malloc failure in libpq calls
* #5233 Out of on_proc_exit slots on guc license change
* #5428 Use consistent snapshots when scanning metadata
* #5499 Do not segfault on large histogram() parameters
* #5470 Ensure superuser perms during copy/move chunk
* #5500 Fix when no FROM clause in continuous aggregate definition
* #5433 Fix join rte in CAggs with joins
* #5556 Fix duplicated entries on timescaledb_experimental.policies view
* #5462 Fix segfault after column drop on compressed table
* #5543 Copy scheduled_jobs list before sorting it
* #5497 Allow named time_bucket arguments in Cagg definition
* #5544 Fix refresh from beginning of Continuous Aggregate with variable time bucket
* #5558 Use regrole for job owner
* #5542 Enable indexscan on uncompressed part of partially compressed chunks

**Thanks**
* @nikolaps for reporting an issue with the COPY fetcher
* @S-imo-n for reporting the issue on Background Worker Scheduler crash
* @geezhu for reporting issue on segfault in historgram()
* @mwahlhuetter for reporting the issue with joins in CAggs
* @mwahlhuetter for reporting issue with duplicated entries on timescaledb_experimental.policies view
* @H25E for reporting error refreshing from beginning of a Continuous Aggregate with variable time bucket
akuzm added a commit that referenced this pull request Apr 19, 2023
 ## 2.10.2 (2023-04-20)

**Bugfixes**
* #5410 Fix file trailer handling in the COPY fetcher
* #5446 Add checks for malloc failure in libpq calls
* #5233 Out of on_proc_exit slots on guc license change
* #5428 Use consistent snapshots when scanning metadata
* #5499 Do not segfault on large histogram() parameters
* #5470 Ensure superuser perms during copy/move chunk
* #5500 Fix when no FROM clause in continuous aggregate definition
* #5433 Fix join rte in CAggs with joins
* #5556 Fix duplicated entries on timescaledb_experimental.policies view
* #5462 Fix segfault after column drop on compressed table
* #5543 Copy scheduled_jobs list before sorting it
* #5497 Allow named time_bucket arguments in Cagg definition
* #5544 Fix refresh from beginning of Continuous Aggregate with variable time bucket
* #5558 Use regrole for job owner
* #5542 Enable indexscan on uncompressed part of partially compressed chunks

**Thanks**
* @nikolaps for reporting an issue with the COPY fetcher
* @S-imo-n for reporting the issue on Background Worker Scheduler crash
* @geezhu for reporting issue on segfault in historgram()
* @mwahlhuetter for reporting the issue with joins in CAggs
* @mwahlhuetter for reporting issue with duplicated entries on timescaledb_experimental.policies view
* @H25E for reporting error refreshing from beginning of a Continuous Aggregate with variable time bucket
akuzm added a commit that referenced this pull request Apr 19, 2023
 ## 2.10.2 (2023-04-20)

**Bugfixes**
* #5410 Fix file trailer handling in the COPY fetcher
* #5446 Add checks for malloc failure in libpq calls
* #5233 Out of on_proc_exit slots on guc license change
* #5428 Use consistent snapshots when scanning metadata
* #5499 Do not segfault on large histogram() parameters
* #5470 Ensure superuser perms during copy/move chunk
* #5500 Fix when no FROM clause in continuous aggregate definition
* #5433 Fix join rte in CAggs with joins
* #5556 Fix duplicated entries on timescaledb_experimental.policies view
* #5462 Fix segfault after column drop on compressed table
* #5543 Copy scheduled_jobs list before sorting it
* #5497 Allow named time_bucket arguments in Cagg definition
* #5544 Fix refresh from beginning of Continuous Aggregate with variable time bucket
* #5558 Use regrole for job owner
* #5542 Enable indexscan on uncompressed part of partially compressed chunks

**Thanks**
* @nikolaps for reporting an issue with the COPY fetcher
* @S-imo-n for reporting the issue on Background Worker Scheduler crash
* @geezhu for reporting issue on segfault in historgram()
* @mwahlhuetter for reporting the issue with joins in CAggs
* @mwahlhuetter for reporting issue with duplicated entries on timescaledb_experimental.policies view
* @H25E for reporting error refreshing from beginning of a Continuous Aggregate with variable time bucket
akuzm added a commit to akuzm/timescaledb that referenced this pull request Apr 19, 2023
 ## 2.10.2 (2023-04-20)

**Bugfixes**
* timescale#5410 Fix file trailer handling in the COPY fetcher
* timescale#5446 Add checks for malloc failure in libpq calls
* timescale#5233 Out of on_proc_exit slots on guc license change
* timescale#5428 Use consistent snapshots when scanning metadata
* timescale#5499 Do not segfault on large histogram() parameters
* timescale#5470 Ensure superuser perms during copy/move chunk
* timescale#5500 Fix when no FROM clause in continuous aggregate definition
* timescale#5433 Fix join rte in CAggs with joins
* timescale#5556 Fix duplicated entries on timescaledb_experimental.policies view
* timescale#5462 Fix segfault after column drop on compressed table
* timescale#5543 Copy scheduled_jobs list before sorting it
* timescale#5497 Allow named time_bucket arguments in Cagg definition
* timescale#5544 Fix refresh from beginning of Continuous Aggregate with variable time bucket
* timescale#5558 Use regrole for job owner
* timescale#5542 Enable indexscan on uncompressed part of partially compressed chunks

**Thanks**
* @nikolaps for reporting an issue with the COPY fetcher
* @S-imo-n for reporting the issue on Background Worker Scheduler crash
* @geezhu for reporting issue on segfault in historgram()
* @mwahlhuetter for reporting the issue with joins in CAggs
* @mwahlhuetter for reporting issue with duplicated entries on timescaledb_experimental.policies view
* @H25E for reporting error refreshing from beginning of a Continuous Aggregate with variable time bucket
akuzm added a commit that referenced this pull request Apr 20, 2023
 ## 2.10.2 (2023-04-20)

**Bugfixes**
* #5410 Fix file trailer handling in the COPY fetcher
* #5446 Add checks for malloc failure in libpq calls
* #5233 Out of on_proc_exit slots on guc license change
* #5428 Use consistent snapshots when scanning metadata
* #5499 Do not segfault on large histogram() parameters
* #5470 Ensure superuser perms during copy/move chunk
* #5500 Fix when no FROM clause in continuous aggregate definition
* #5433 Fix join rte in CAggs with joins
* #5556 Fix duplicated entries on timescaledb_experimental.policies view
* #5462 Fix segfault after column drop on compressed table
* #5543 Copy scheduled_jobs list before sorting it
* #5497 Allow named time_bucket arguments in Cagg definition
* #5544 Fix refresh from beginning of Continuous Aggregate with variable time bucket
* #5558 Use regrole for job owner
* #5542 Enable indexscan on uncompressed part of partially compressed chunks

**Thanks**
* @nikolaps for reporting an issue with the COPY fetcher
* @S-imo-n for reporting the issue on Background Worker Scheduler crash
* @geezhu for reporting issue on segfault in historgram()
* @mwahlhuetter for reporting the issue with joins in CAggs
* @mwahlhuetter for reporting issue with duplicated entries on timescaledb_experimental.policies view
* @H25E for reporting error refreshing from beginning of a Continuous Aggregate with variable time bucket
akuzm added a commit that referenced this pull request Apr 20, 2023
 ## 2.10.2 (2023-04-20)

**Bugfixes**
* #5410 Fix file trailer handling in the COPY fetcher
* #5446 Add checks for malloc failure in libpq calls
* #5233 Out of on_proc_exit slots on guc license change
* #5428 Use consistent snapshots when scanning metadata
* #5499 Do not segfault on large histogram() parameters
* #5470 Ensure superuser perms during copy/move chunk
* #5500 Fix when no FROM clause in continuous aggregate definition
* #5433 Fix join rte in CAggs with joins
* #5556 Fix duplicated entries on timescaledb_experimental.policies view
* #5462 Fix segfault after column drop on compressed table
* #5543 Copy scheduled_jobs list before sorting it
* #5497 Allow named time_bucket arguments in Cagg definition
* #5544 Fix refresh from beginning of Continuous Aggregate with variable time bucket
* #5558 Use regrole for job owner
* #5542 Enable indexscan on uncompressed part of partially compressed chunks

**Thanks**
* @nikolaps for reporting an issue with the COPY fetcher
* @S-imo-n for reporting the issue on Background Worker Scheduler crash
* @geezhu for reporting issue on segfault in historgram()
* @mwahlhuetter for reporting the issue with joins in CAggs
* @mwahlhuetter for reporting issue with duplicated entries on timescaledb_experimental.policies view
* @H25E for reporting error refreshing from beginning of a Continuous Aggregate with variable time bucket
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Copy fetcher creates NULL values
4 participants