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

Release 2.2.1 - release branch #3173

Merged
merged 35 commits into from May 5, 2021
Merged

Conversation

mkindahl
Copy link
Contributor

@mkindahl mkindahl commented Apr 29, 2021

This is the list of commits:

$ git log --no-decorate --oneline 2.2.0..
cc6c5233 Release 2.2.1
ea50fbe2 Add 2.2.0 to update test scripts
b97cfc25 Fix segfault in calculate_chunk_interval
3f0a9c1a Fix wrong datatype in integer based retention policy
7f81c64e Add a few miscellaneous code fixes
3a6bd974 Add changelog entry for #3169
fd7d1d70 Fix incorrect type cast in compression policy
b4e1610c Use no-unused-command-line-argument flag
8d3951fa Add coredump support for 32bit ci runs
d5f17556 Inherit CFLAGS from postgresql
b470dad9 Fix fdw_relinfo_get assertion failure on DELETE
b92b1af6 Copy recreated object permissions on update
20459fe1 Fix flaky dist_hypertable test
e0fc2bf5 Add GitHub action for posting daily CI summary
6f422a5d Fix use after free in cache
6977de02 Remove slack notification from individual workflows
fe1fc4af Add update smoke test script
26a7752e Make SELECT DISTINCT handle non-var targetlists
f3bbde8c Skip ChunkAppend if AppendPath has no children
def6534d Fix SkipScan for IndexPaths without pathkeys
45f1b2e2 Fix flaky pg_dump test
ba41b562 Run code style regression test on all PRs
f454522d Ignore generated SQL test files from templates
3b1582c6 Add "SELECT DISTINCT" pushdown in multi-node
753ebcb6 Increase number of connections for tsl tests
2c645b6d Fix SkipScan path generation with expressions
84b945b1 Fix REINDEX TABLE for distributed hypertables
3600c066 Fix crash while using REINDEX TABLE CONCURRENTLY
973e37c1 Fix windows CI PG13 failure
8c7bd485 Use %u to format Oid instead of %d
063daa15 Move plan nodes to nodes subdirectory
5a59ba7b Fix use after free in chunk_api_get_chunk_stats
1b8b97df Fix use after free in add_reorder_policy
776efc4a Fix CMAKE_BUILD_TYPE check
6ad8a2b9 Use commit date in get_git_commit()

@mkindahl mkindahl requested a review from a team as a code owner April 29, 2021 09:15
@mkindahl mkindahl requested review from afiskon, nikkhils, mfundul and k-rus and removed request for a team April 29, 2021 09:15
@codecov
Copy link

codecov bot commented Apr 29, 2021

Codecov Report

Merging #3173 (b97cfc2) into 2.2.x (512ae89) will increase coverage by 0.04%.
The diff coverage is 81.66%.

❗ Current head b97cfc2 differs from pull request most recent head cc6c523. Consider uploading reports for the commit cc6c523 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##            2.2.x    #3173      +/-   ##
==========================================
+ Coverage   90.17%   90.21%   +0.04%     
==========================================
  Files         215      215              
  Lines       35301    35319      +18     
==========================================
+ Hits        31831    31863      +32     
+ Misses       3470     3456      -14     
Impacted Files Coverage Δ
src/copy.c 87.91% <ø> (+0.51%) ⬆️
src/hypertable_restrict_info.c 95.58% <0.00%> (ø)
src/init.c 63.63% <ø> (ø)
src/nodes/chunk_dispatch.c 98.21% <ø> (ø)
src/nodes/chunk_dispatch_plan.c 100.00% <ø> (ø)
src/nodes/chunk_dispatch_state.c 95.29% <ø> (ø)
src/nodes/chunk_insert_state.c 97.66% <ø> (ø)
.../constraint_aware_append/constraint_aware_append.c 92.50% <ø> (ø)
src/nodes/hypertable_insert.c 96.38% <ø> (ø)
src/plan_agg_bookend.c 92.30% <ø> (ø)
... and 52 more

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 512ae89...cc6c523. Read the comment docs.

Copy link
Contributor

@k-rus k-rus left a comment

Choose a reason for hiding this comment

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

TimescaleDB fails to compile in some tests. There is a PR to master, which is supposed to fix the issue.

@mkindahl
Copy link
Contributor Author

TimescaleDB fails to compile in some tests. There is a PR to master, which is supposed to fix the issue.

Yes, I noted the failures.

@mkindahl
Copy link
Contributor Author

Following update failure is weird as well. It suggests that the updated version is missing a column that the pg_dump has, which is just weird.

--- /tmp/tmp.wOKiLRlYMH/timescaledb-updated-3539.out	2021-04-29 09:27:10.293932120 +0000
+++ /tmp/tmp.wOKiLRlYMH/timescaledb-clean-restore-3539.out	2021-04-29 09:27:14.353987676 +0000
@@ -57,6 +57,7 @@
  table_name          | name    |           | not null |                                                        | plain   |              | 
  compressed_chunk_id | integer |           |          |                                                        | plain   |              | 
  dropped             | boolean |           | not null | false                                                  | plain   |              | 
+ status              | integer |           | not null | 0                                                      | plain   |              | 
 Indexes:
     "chunk_pkey" PRIMARY KEY, btree (id)
     "chunk_compressed_chunk_id_idx" btree (compressed_chunk_id)
@@ -11088,7 +11089,7 @@
                   seqrelid                   | nextval | seqstart | seqincrement |       seqmax        | seqmin 
 ---------------------------------------------+---------+----------+--------------+---------------------+--------
  _timescaledb_catalog.chunk_constraint_name  |      14 |        1 |            1 | 9223372036854775807 |      1
- _timescaledb_catalog.chunk_id_seq           |      87 |        1 |            1 |          2147483647 |      1
+ _timescaledb_catalog.chunk_id_seq           |      87 |        1 |            1 | 9223372036854775807 |      1
  _timescaledb_catalog.dimension_id_seq       |      22 |        1 |            1 |          2147483647 |      1
  _timescaledb_catalog.dimension_slice_id_seq |     169 |        1 |            1 |          2147483647 |      1
  _timescaledb_catalog.hypertable_id_seq      |      21 |        1 |            1 | 9223372036854775807 |      1

@mkindahl mkindahl force-pushed the release-2.2.x branch 3 times, most recently from 2210659 to 80a39d0 Compare April 29, 2021 12:59
svenklemm and others added 17 commits April 29, 2021 15:07
get_git_commit used the author date instead of the commit date
when showing information about the commit. For the purpose of
the function the author date is not that significant and when
the commit to the branch actually happened is much more relevant.
Possible values are Release, Debug, RelWithDebInfo and MinSizeRel
https://llvm.org/docs/CMake.html#frequently-used-cmake-variables
The hypertable object in policy_reorder_add was still used after
releasing the cache leading to occasional segfaults in CI.
Reorg source code to create a nodes subdirectory.
Move plan nodes to this drectory.
Since Oid is unsigned int we have to use %u to print it otherwise
oids >= 2^31 will not work correctly. This also switches the places
that print type oid to use format helper functions to resolve the
oids.
Installing the postgres13 windows package pulled in an install
of vcredist140 which requires a reboot making chocolatey return
with a non-zero exit code to signal the required reboot. This
patch changes chocolatey to no longer give individual packages
control over chocolatey exit code. This patch also changes
chocolatey to longer show package download progress information
to make the CI log less spammy.
CONCURRENTLY option of the REINDEX TABLE is not supported and
leads to a crash when using with hypertable. Block it and print
the error message.

Fix timescale#3122
In case of distributed hypertable do not propagate the REINDEX
command to chunks. Allow command to be passed for remote execution
on data nodes.

Fixes timescale#3018
In DISTINCT when deciding about adding a SkipScan path we try
to match the PathKey to the targetlist which would fail if
PathKey was an expression but the index not an expression
index. The check for the DISTINCT being a Var would only happen
after this check leading to a "skip column not found in targetlist"
error under these circumstances.
This only affects DISTINCT queries with targetlist with 1 non-Const
entry that is an expression as others would get rejected earlier when
checking whether to build SkipScan path.

Fixes timescale#3134
Since we run 20 tests in parallel in the tsl tests multiple
multinode tests might exhaust the number of connections the
test instance allowed. This patch changes the number of connections
to 200 for regresscheck-t.
Construct "SELECT DISTINCT target_list" or "SELECT DISTINCT ON (col1,
col..) target_list" statement to push down the DISTINCT clause to the
remote side.

We only allow references to basic "Vars" or constants in the DISTINCT
exprs

So, "SELECT DISTINCT col1" is fine but "SELECT DISTINCT 2*col1" is not.

"SELECT DISTINCT col1, 'const1', NULL, col2" which is a mix of column
references and constants is also supported. Everything else is not
supported.

This pushdown also needs to work when
timescaledb.enable_per_data_node_queries is disabled.

All existing test cases in which "SELECT DISTINCT" is now being pushed
down have been modified. New test cases have been added to check that
the remote side uses "Skip Scans" as is suitable in some cases.
`index` test was moved into SQL test template in timescale#3123 and
`generated_columns` - in timescale#2927. They are added to relevant gitignore.
Checking code formatting and other simple code style test is good to
perform on PRs to feature branches, not only the master branch. This
commit removes the limitation to perform tests only on PR to the
master.
This patch fixes the flaky pg_dump test by switching off the current
database before using it as template. It also disables background
job scheduling instead of stopping them before creating the new
database as stopping bgw jobs might not happen immediately.

Fixes timescale#2847
The SkipScan code assumed IndexPaths on ordered indexes always
have pathkeys which is not true when the pathkeys are not useful
for the query, leading to a segfault for those queries.
Since postgres has optimizations for Append paths without children
we do not replace those paths with ChunkAppend. While this will
prevent ChunkAppendPaths without children during planning, in the
executor it is still possible for ChunkAppend to have no children
as run-time exclusion may remove all of them.
svenklemm and others added 12 commits April 29, 2021 15:09
The patch removes the Slack notification of individual workflows
in favor of the Slack summary message
When calling hash_search with HASH_REMOVE the returned pointer should not
be dereferenced because it returns a dangling pointer
This patch adds a GitHub action that posts all failed CI jobs in
the last 24 hours with link to the failed job.
The chunks of the datanodes seem to contain very few rows. That can
cause PostgreSQL to choose a SeqScan plan in some cases. Add a few
more rows to make it choose IndexScan all of the time.
Tables, indexes, and sequences that are recreated as part of an update
does not propagate permissions to the recreated object. This commit
fixes that by saving away the permissions in `pg_class` temporarily and
then copying them back into the `pg_class` table.

If internal objects are created or re-created, they get the wrong
initial privileges, which result in privileges not being dumped when
using `pg_dump`. Save away the privileges before starting the update
and restore them afterwards to make sure that the privileges are
maintained over the update.

For new objects, we use the initial privileges of the `chunk` metadata
table, which should always have correct initial privileges.

Fixes timescale#3078
The code in fdw_relinfo_get assumed rel->fdw_private and
rel->fdw_private->fdw_relation_info were always allocated
which is not true for DELETEs for instance.
There are quite a few compiler flags that need to be identical
between postgresql and timescaledb otherwise there will be
seemingly unexplainable failures during execution.
We inherit all CFLAGS from postgresql except those starting
with -W as we have our own set of warning options.
This patch adds coredump support for the 32bit CI runs. This patch
switches 32bit run to the debian base image and install postgres
from PGDG instead of using the alpine image so we don't have to
compile postgres ourselves and can just install the debugsymbol
package for postgres.
We started inheriting CFLAGS from postgresql via ba51adc. Although we
remove "-W" flags, we still end up accepting other options like "-L"
from postgresql. This can cause compilation to fail with compilers
like Clang which enable reporting of errors on unused command line
arguments.

This patch conditionally adds -Wno-unused-command-line-argument in
our cmake files to avoid unhelpful unused-command-line-argument
warnings/errors.
Fix incorrect type cast when reading int64 compress after from compression policy

Fixes timescale#3009

Signed-off-by: Xin Li <xin.li@hedera.com>
1) Quell a compiler warning about uninitialized use of "varno"
variable.

2) Fix ts_chunk_get_by_name to return early if rogue schema/table args
are passed in.
svenklemm and others added 4 commits May 4, 2021 14:59
When reading the configuration for retention policy of integer
based hypertables int32 was used instead of int64.

Fixes timescale#2877
Block calling calculate_chunk_interval on distributed hypertables
to prevent segfault when finding min and max as distributed chunks
have no tableam since they are foreign tables.
This maintenance release contains bugfixes since the 2.2.0 release. We
deem it high priority for upgrading.

This release extends Skip Scan to multinode by enabling the pushdown
of `DISTINCT` to data nodes. It also fixes a number of bugs in the
implementation of Skip Scan, in distributed hypertables, in creation
of indexes, in compression, and in policies.

**Features**
* timescale#3113 Pushdown "SELECT DISTINCT" in multi-node to allow use of Skip
  Scan

**Bugfixes**
* timescale#3101 Use commit date in `get_git_commit()`
* timescale#3102 Fix `REINDEX TABLE` for distributed hypertables
* timescale#3104 Fix use after free in `add_reorder_policy`
* timescale#3106 Fix use after free in `chunk_api_get_chunk_stats`
* timescale#3109 Copy recreated object permissions on update
* timescale#3111 Fix `CMAKE_BUILD_TYPE` check
* timescale#3112 Use `%u` to format Oid instead of `%d`
* timescale#3118 Fix use after free in cache
* timescale#3123 Fix crash while using `REINDEX TABLE CONCURRENTLY`
* timescale#3135 Fix SkipScan path generation in `DISTINCT` queries
  with expressions
* timescale#3146 Fix SkipScan for IndexPaths without pathkeys
* timescale#3147 Skip ChunkAppend if AppendPath has no children
* timescale#3148 Make `SELECT DISTINCT` handle non-var targetlists
* timescale#3151 Fix `fdw_relinfo_get` assertion failure on `DELETE`
* timescale#3155 Inherit `CFLAGS` from PostgreSQL
* timescale#3169 Fix incorrect type cast in compression policy
* timescale#3183 Fix segfault in calculate_chunk_interval
* timescale#3185 Fix wrong datatype in integer based retention policy

**Thanks**
* @Dead2, @dv8472 and @einsibjarni for reporting an issue with
  multinode queries and views
* @hperez75 for reporting an issue with Skip Scan
* @nathanloisel for reporting an issue with compression on hypertables
  with integer-based timestamps
* @xin-hedera for fixing an issue with compression on hypertables with
  integer-based timestamps
@mkindahl mkindahl removed the request for review from k-rus May 5, 2021 10:08
@mkindahl
Copy link
Contributor Author

mkindahl commented May 5, 2021

Following update failure is weird as well. It suggests that the updated version is missing a column that the pg_dump has, which is just weird.

This turned out to be because I had included the commit that adds the current version to the update tests. Since we haven't bumped the version when only the commits are cherry picked, it tried to run an update from 2.2.0 to 2.2.0, which does not run the update script. Hence the diff.

@mkindahl mkindahl self-assigned this May 5, 2021
@mkindahl mkindahl changed the title Release 2.2.1 - Cherry picked commits Release 2.2.1 May 5, 2021
@mkindahl mkindahl changed the title Release 2.2.1 Release 2.2.1 - release branch May 5, 2021
@mkindahl mkindahl merged commit 95a9503 into timescale:2.2.x May 5, 2021
@mkindahl mkindahl deleted the release-2.2.x branch May 5, 2021 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants