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 memory context bug executing TRUNCATE. #3626

Merged
merged 1 commit into from
Oct 6, 2021

Conversation

fabriziomello
Copy link
Contributor

@fabriziomello fabriziomello commented Sep 29, 2021

Inside the process_truncate function is created a new relations list
removing the distributed hypertables and this new list is assigned to
the current statement relation list. This new list is allocated into
the PortalContext that is destroyed at the end of the statement
execution.

The problem arise on the subsequent TRUNCATE call because the
compiled plpgsql code is cached into another memory context and the
elements of the relations inside this cache is pointing to an invalid
memory area because the PortalContext is destroyed.

Fixed it by allocating the new relations list to the same memory
context of the original list.

Fixes #3580, fixes #3622, fixes #3182

@codecov
Copy link

codecov bot commented Sep 29, 2021

Codecov Report

Merging #3626 (f077220) into master (ef0ff45) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3626      +/-   ##
==========================================
- Coverage   90.82%   90.78%   -0.04%     
==========================================
  Files         212      212              
  Lines       36534    36547      +13     
==========================================
- Hits        33182    33180       -2     
- Misses       3352     3367      +15     
Impacted Files Coverage Δ
src/chunk_index.c 94.77% <100.00%> (+0.05%) ⬆️
src/process_utility.c 94.44% <100.00%> (+0.03%) ⬆️
src/bgw/scheduler.c 83.04% <0.00%> (-3.51%) ⬇️
src/import/planner.c 63.63% <0.00%> (-0.68%) ⬇️
src/bgw/job.c 91.97% <0.00%> (-0.34%) ⬇️
tsl/src/bgw_policy/job.c 96.85% <0.00%> (-0.01%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f64e11...f077220. Read the comment docs.

@fabriziomello fabriziomello force-pushed the issue/3580 branch 3 times, most recently from 8ecbf37 to 42f8b31 Compare September 30, 2021 21:32
@fabriziomello fabriziomello self-assigned this Sep 30, 2021
@fabriziomello fabriziomello marked this pull request as ready for review September 30, 2021 21:33
@fabriziomello fabriziomello requested a review from a team as a code owner September 30, 2021 21:33
@fabriziomello fabriziomello requested review from svenklemm, akuzm, duncan-tsdb, gayyappan, mkindahl and a team and removed request for a team, akuzm and duncan-tsdb September 30, 2021 21:33
Copy link
Contributor

@mkindahl mkindahl left a comment

Choose a reason for hiding this comment

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

We do not use a . last in the title line.

test/expected/truncate.out Outdated Show resolved Hide resolved
tsl/src/remote/dist_ddl.c Outdated Show resolved Hide resolved
@fabriziomello fabriziomello force-pushed the issue/3580 branch 2 times, most recently from 8cd3fd7 to 0f44971 Compare October 1, 2021 17:31
src/process_utility.c Outdated Show resolved Hide resolved
@fabriziomello
Copy link
Contributor Author

We do not use a . last in the title line.

Fixed!

@fabriziomello
Copy link
Contributor Author

Fixed it by allocating the new relations list to the TopMemoryContext just when it changes.

src/process_utility.c Outdated Show resolved Hide resolved
Copy link
Member

@svenklemm svenklemm left a comment

Choose a reason for hiding this comment

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

Maybe we can make the memorycontext switches more surgical and only run the stuff that actually needs to be in the other memorycontext in that context instead of only switching once at top of function.

@fabriziomello
Copy link
Contributor Author

Maybe we can make the memorycontext switches more surgical and only run the stuff that actually needs to be in the other memorycontext in that context instead of only switching once at top of function.

But I did it... did you see? https://github.com/timescale/timescaledb/pull/3626/files#diff-9816cecf26e0199a4235ff8b1ac830af68a06ebee40a46421d4e877c81c4e16bR1026

@fabriziomello fabriziomello force-pushed the issue/3580 branch 2 times, most recently from e309de1 to 962420a Compare October 6, 2021 13:11
@NunoFilipeSantos NunoFilipeSantos added this to the TimescaleDB 2.5 milestone Oct 6, 2021
@fabriziomello
Copy link
Contributor Author

@svenklemm also fixed the makeRangeVar to happen in the same memory context of the list

Copy link
Member

@svenklemm svenklemm left a comment

Choose a reason for hiding this comment

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

Please don't forget to add this PR to Bugfixes in CHANGELOG.md and also add thanks for the persons who reported it

Inside the `process_truncate` function is created a new relations list
removing the distributed hypertables and this new list is assigned to
the current statement relation list. This new list is allocated into
the `PortalContext` that is destroyed at the end of the statement
execution.

The problem arise on the subsequent `TRUNCATE` call because the
compiled plpgsql code is cached into another memory context and the
elements of the relations inside this cache is pointing to an invalid
memory area because the `PortalContext` is destroyed.

Fixed it by allocating the new relations list to the same memory
context of the original list.

Fixes timescale#3580, fixes timescale#3622, fixes timescale#3182
@fabriziomello fabriziomello merged commit 74ca546 into timescale:master Oct 6, 2021
fabriziomello added a commit to fabriziomello/timescaledb that referenced this pull request Oct 27, 2021
This release adds major new features since the 2.4.2 release.
We deem it moderate priority for upgrading.

This release includes these noteworthy features:

* Continuous Aggregates for Distributed Hypertables
* Support for PostgreSQL 14
* Experimental: Support for timezones in `time_bucket_ng()`, including
the `origin` argument

This release also includes several bug fixes.

**Features**
* timescale#3034 Add support for PostgreSQL 14
* timescale#3435 Add continuous aggregates for distributed hypertables
* timescale#3505 Add support for timezones in `time_bucket_ng()`
* timescale#3598 Improve evaluation of stable functions such as now() on access node
* timescale#3717 Support transparent decompression on individual chunks

**Bugfixes**
* timescale#3580 Fix memory context bug executing TRUNCATE
* timescale#3592 Allow alter column type on distributed hypertable
* timescale#3618 Fix execution of refresh_caggs from user actions
* timescale#3625 Add shared dependencies when creating chunk
* timescale#3626 Fix memory context bug executing TRUNCATE
* timescale#3627 Schema qualify UDTs in multi-node
* timescale#3638 Allow owner change of a data node
* timescale#3654 Fix index attnum mapping in reorder_chunk
* timescale#3661 Fix SkipScan path generation with constant DISTINCT column
* timescale#3667 Fix compress_policy for multi txn handling
* timescale#3673 Fix distributed hypertable DROP within a procedure
* timescale#3701 Allow anyone to use size utilities on distributed hypertables
* timescale#3708 Fix crash in get_aggsplit
* timescale#3709 Fix ordered append pathkey check
* timescale#3712 Fix GRANT/REVOKE ALL IN SCHEMA handling
* timescale#3724 Fix inserts into compressed chunks on hypertables with caggs
* timescale#3727 Fix DirectFunctionCall crash in distributed_exec
* timescale#3728 Fix SkipScan with varchar column
* timescale#3733 Fix ANALYZE crash with custom statistics for custom types
* timescale#3747 Always reset expr context in DecompressChunk

**Thanks**
* @binakot and @sebvett for reporting an issue with DISTINCT queries
* @hardikm10, @DavidPavlicek and @pafiti for reporting bugs on TRUNCATE
* @mjf for reporting an issue with ordered append and JOINs
* @phemmer for reporting the issues on multinode with aggregate queries and evaluation of now()
* @abolognino for reporting an issue with INSERTs into compressed hypertables that have cagg
* @tanglebones for reporting the ANALYZE crash with custom types on multinode
@fabriziomello fabriziomello mentioned this pull request Oct 27, 2021
fabriziomello added a commit to fabriziomello/timescaledb that referenced this pull request Oct 27, 2021
This release adds major new features since the 2.4.2 release.
We deem it moderate priority for upgrading.

This release includes these noteworthy features:

* Continuous Aggregates for Distributed Hypertables
* Support for PostgreSQL 14
* Experimental: Support for timezones in `time_bucket_ng()`, including
the `origin` argument

This release also includes several bug fixes.

**Features**
* timescale#3034 Add support for PostgreSQL 14
* timescale#3435 Add continuous aggregates for distributed hypertables
* timescale#3505 Add support for timezones in `time_bucket_ng()`

**Bugfixes**
* timescale#3580 Fix memory context bug executing TRUNCATE
* timescale#3592 Allow alter column type on distributed hypertable
* timescale#3598 Improve evaluation of stable functions such as now() on access
node
* timescale#3618 Fix execution of refresh_caggs from user actions
* timescale#3625 Add shared dependencies when creating chunk
* timescale#3626 Fix memory context bug executing TRUNCATE
* timescale#3627 Schema qualify UDTs in multi-node
* timescale#3638 Allow owner change of a data node
* timescale#3654 Fix index attnum mapping in reorder_chunk
* timescale#3661 Fix SkipScan path generation with constant DISTINCT column
* timescale#3667 Fix compress_policy for multi txn handling
* timescale#3673 Fix distributed hypertable DROP within a procedure
* timescale#3701 Allow anyone to use size utilities on distributed hypertables
* timescale#3708 Fix crash in get_aggsplit
* timescale#3709 Fix ordered append pathkey check
* timescale#3712 Fix GRANT/REVOKE ALL IN SCHEMA handling
* timescale#3717 Support transparent decompression on individual chunks
* timescale#3724 Fix inserts into compressed chunks on hypertables with caggs
* timescale#3727 Fix DirectFunctionCall crash in distributed_exec
* timescale#3728 Fix SkipScan with varchar column
* timescale#3733 Fix ANALYZE crash with custom statistics for custom types
* timescale#3747 Always reset expr context in DecompressChunk

**Thanks**
* @binakot and @sebvett for reporting an issue with DISTINCT queries
* @hardikm10, @DavidPavlicek and @pafiti for reporting bugs on TRUNCATE
* @mjf for reporting an issue with ordered append and JOINs
* @phemmer for reporting the issues on multinode with aggregate queries and evaluation of now()
* @abolognino for reporting an issue with INSERTs into compressed hypertables that have cagg
* @tanglebones for reporting the ANALYZE crash with custom types on multinode
fabriziomello added a commit that referenced this pull request Oct 27, 2021
This release adds major new features since the 2.4.2 release.
We deem it moderate priority for upgrading.

This release includes these noteworthy features:

* Continuous Aggregates for Distributed Hypertables
* Support for PostgreSQL 14
* Experimental: Support for timezones in `time_bucket_ng()`, including
the `origin` argument

This release also includes several bug fixes.

**Features**
* #3034 Add support for PostgreSQL 14
* #3435 Add continuous aggregates for distributed hypertables
* #3505 Add support for timezones in `time_bucket_ng()`

**Bugfixes**
* #3580 Fix memory context bug executing TRUNCATE
* #3592 Allow alter column type on distributed hypertable
* #3598 Improve evaluation of stable functions such as now() on access
node
* #3618 Fix execution of refresh_caggs from user actions
* #3625 Add shared dependencies when creating chunk
* #3626 Fix memory context bug executing TRUNCATE
* #3627 Schema qualify UDTs in multi-node
* #3638 Allow owner change of a data node
* #3654 Fix index attnum mapping in reorder_chunk
* #3661 Fix SkipScan path generation with constant DISTINCT column
* #3667 Fix compress_policy for multi txn handling
* #3673 Fix distributed hypertable DROP within a procedure
* #3701 Allow anyone to use size utilities on distributed hypertables
* #3708 Fix crash in get_aggsplit
* #3709 Fix ordered append pathkey check
* #3712 Fix GRANT/REVOKE ALL IN SCHEMA handling
* #3717 Support transparent decompression on individual chunks
* #3724 Fix inserts into compressed chunks on hypertables with caggs
* #3727 Fix DirectFunctionCall crash in distributed_exec
* #3728 Fix SkipScan with varchar column
* #3733 Fix ANALYZE crash with custom statistics for custom types
* #3747 Always reset expr context in DecompressChunk

**Thanks**
* @binakot and @sebvett for reporting an issue with DISTINCT queries
* @hardikm10, @DavidPavlicek and @pafiti for reporting bugs on TRUNCATE
* @mjf for reporting an issue with ordered append and JOINs
* @phemmer for reporting the issues on multinode with aggregate queries and evaluation of now()
* @abolognino for reporting an issue with INSERTs into compressed hypertables that have cagg
* @tanglebones for reporting the ANALYZE crash with custom types on multinode
fabriziomello added a commit that referenced this pull request Oct 27, 2021
This release adds major new features since the 2.4.2 release.
We deem it moderate priority for upgrading.

This release includes these noteworthy features:

* Continuous Aggregates for Distributed Hypertables
* Support for PostgreSQL 14
* Experimental: Support for timezones in `time_bucket_ng()`, including
the `origin` argument

This release also includes several bug fixes.

**Features**
* #3034 Add support for PostgreSQL 14
* #3435 Add continuous aggregates for distributed hypertables
* #3505 Add support for timezones in `time_bucket_ng()`

**Bugfixes**
* #3580 Fix memory context bug executing TRUNCATE
* #3592 Allow alter column type on distributed hypertable
* #3598 Improve evaluation of stable functions such as now() on access
node
* #3618 Fix execution of refresh_caggs from user actions
* #3625 Add shared dependencies when creating chunk
* #3626 Fix memory context bug executing TRUNCATE
* #3627 Schema qualify UDTs in multi-node
* #3638 Allow owner change of a data node
* #3654 Fix index attnum mapping in reorder_chunk
* #3661 Fix SkipScan path generation with constant DISTINCT column
* #3667 Fix compress_policy for multi txn handling
* #3673 Fix distributed hypertable DROP within a procedure
* #3701 Allow anyone to use size utilities on distributed hypertables
* #3708 Fix crash in get_aggsplit
* #3709 Fix ordered append pathkey check
* #3712 Fix GRANT/REVOKE ALL IN SCHEMA handling
* #3717 Support transparent decompression on individual chunks
* #3724 Fix inserts into compressed chunks on hypertables with caggs
* #3727 Fix DirectFunctionCall crash in distributed_exec
* #3728 Fix SkipScan with varchar column
* #3733 Fix ANALYZE crash with custom statistics for custom types
* #3747 Always reset expr context in DecompressChunk

**Thanks**
* @binakot and @sebvett for reporting an issue with DISTINCT queries
* @hardikm10, @DavidPavlicek and @pafiti for reporting bugs on TRUNCATE
* @mjf for reporting an issue with ordered append and JOINs
* @phemmer for reporting the issues on multinode with aggregate queries
and evaluation of now()
* @abolognino for reporting an issue with INSERTs into compressed
hypertables that have cagg
* @tanglebones for reporting the ANALYZE crash with custom types on
multinode
fabriziomello added a commit that referenced this pull request Oct 27, 2021
This release adds major new features since the 2.4.2 release.
We deem it moderate priority for upgrading.

This release includes these noteworthy features:

* Continuous Aggregates for Distributed Hypertables
* Support for PostgreSQL 14
* Experimental: Support for timezones in `time_bucket_ng()`, including
the `origin` argument

This release also includes several bug fixes.

**Features**
* #3034 Add support for PostgreSQL 14
* #3435 Add continuous aggregates for distributed hypertables
* #3505 Add support for timezones in `time_bucket_ng()`

**Bugfixes**
* #3580 Fix memory context bug executing TRUNCATE
* #3592 Allow alter column type on distributed hypertable
* #3598 Improve evaluation of stable functions such as now() on access
node
* #3618 Fix execution of refresh_caggs from user actions
* #3625 Add shared dependencies when creating chunk
* #3626 Fix memory context bug executing TRUNCATE
* #3627 Schema qualify UDTs in multi-node
* #3638 Allow owner change of a data node
* #3654 Fix index attnum mapping in reorder_chunk
* #3661 Fix SkipScan path generation with constant DISTINCT column
* #3667 Fix compress_policy for multi txn handling
* #3673 Fix distributed hypertable DROP within a procedure
* #3701 Allow anyone to use size utilities on distributed hypertables
* #3708 Fix crash in get_aggsplit
* #3709 Fix ordered append pathkey check
* #3712 Fix GRANT/REVOKE ALL IN SCHEMA handling
* #3717 Support transparent decompression on individual chunks
* #3724 Fix inserts into compressed chunks on hypertables with caggs
* #3727 Fix DirectFunctionCall crash in distributed_exec
* #3728 Fix SkipScan with varchar column
* #3733 Fix ANALYZE crash with custom statistics for custom types
* #3747 Always reset expr context in DecompressChunk

**Thanks**
* @binakot and @sebvett for reporting an issue with DISTINCT queries
* @hardikm10, @DavidPavlicek and @pafiti for reporting bugs on TRUNCATE
* @mjf for reporting an issue with ordered append and JOINs
* @phemmer for reporting the issues on multinode with aggregate queries
and evaluation of now()
* @abolognino for reporting an issue with INSERTs into compressed
hypertables that have cagg
* @tanglebones for reporting the ANALYZE crash with custom types on
multinode
fabriziomello added a commit that referenced this pull request Oct 27, 2021
This release adds major new features since the 2.4.2 release.
We deem it moderate priority for upgrading.

This release includes these noteworthy features:

* Continuous Aggregates for Distributed Hypertables
* Support for PostgreSQL 14
* Experimental: Support for timezones in `time_bucket_ng()`, including
the `origin` argument

This release also includes several bug fixes.

**Features**
* #3034 Add support for PostgreSQL 14
* #3435 Add continuous aggregates for distributed hypertables
* #3505 Add support for timezones in `time_bucket_ng()`

**Bugfixes**
* #3580 Fix memory context bug executing TRUNCATE
* #3592 Allow alter column type on distributed hypertable
* #3598 Improve evaluation of stable functions such as now() on access
node
* #3618 Fix execution of refresh_caggs from user actions
* #3625 Add shared dependencies when creating chunk
* #3626 Fix memory context bug executing TRUNCATE
* #3627 Schema qualify UDTs in multi-node
* #3638 Allow owner change of a data node
* #3654 Fix index attnum mapping in reorder_chunk
* #3661 Fix SkipScan path generation with constant DISTINCT column
* #3667 Fix compress_policy for multi txn handling
* #3673 Fix distributed hypertable DROP within a procedure
* #3701 Allow anyone to use size utilities on distributed hypertables
* #3708 Fix crash in get_aggsplit
* #3709 Fix ordered append pathkey check
* #3712 Fix GRANT/REVOKE ALL IN SCHEMA handling
* #3717 Support transparent decompression on individual chunks
* #3724 Fix inserts into compressed chunks on hypertables with caggs
* #3727 Fix DirectFunctionCall crash in distributed_exec
* #3728 Fix SkipScan with varchar column
* #3733 Fix ANALYZE crash with custom statistics for custom types
* #3747 Always reset expr context in DecompressChunk

**Thanks**
* @binakot and @sebvett for reporting an issue with DISTINCT queries
* @hardikm10, @DavidPavlicek and @pafiti for reporting bugs on TRUNCATE
* @mjf for reporting an issue with ordered append and JOINs
* @phemmer for reporting the issues on multinode with aggregate queries
and evaluation of now()
* @abolognino for reporting an issue with INSERTs into compressed
hypertables that have cagg
* @tanglebones for reporting the ANALYZE crash with custom types on
multinode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants