Fix snapshot usage in CAgg invalidation scanner

[jnidzwetzki]

So far, the scanner in get_lowest_invalidated_time_for_hypertable() used a Snapshot that includes the data of all committed transactions (and not only the transactions that are started before the snapshot was created - SNAPSHOT_SELF). This could lead to a situation where we see the old and the new value of a parallel updated tuple. Since get_lowest_invalidated_time_for_hypertable() expects exactly one tuple to be found during the scan, we end up with an error. Since this is performed in our insert path (i.e., it is executed by the invalidation trigger), this error could reject inserts on the hypertable (ERROR: more than one invalidation watermark found).

---

Should be merged after: #6678

[fabriziomello]

Wondering if we should do the same in https://github.com/timescale/timescaledb/blob/main/tsl/src/continuous_aggs/invalidation_threshold.c#L163

[jnidzwetzki]

Yes, we should do it. However, this is already done in this pending PR: #6672. There, we use the latest snapshot to see parallel updates.

[akuzm]

I'm a little confused about this part -- if session 2 does a backfill into the range that was refreshed by session 1, what will be the final watermark value?

[jnidzwetzki]

In an ideal world, we should see the most recent timestamp (i.e., if a parallel refresh runs in parallel and moves the watermark, it should be guaranteed that we see this update. Otherwise we might lose an invalidation since we believe we are > watermark). Said this, we have to avoid strong locks since this code runs on every transaction commit.

We have a rewrite of this code path on our agenda. The goal is to get rid of the complete invalidation trigger and perform this more asynchronously.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (59f50f2) 80.06% compared to head (6fb552c) 81.40%.
Report is 21 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6677      +/-   ##
==========================================
+ Coverage   80.06%   81.40%   +1.34%     
==========================================
  Files         190      191       +1     
  Lines       37181    36422     -759     
  Branches     9450     9465      +15     
==========================================
- Hits        29770    29651     -119     
- Misses       2997     2998       +1     
+ Partials     4414     3773     -641     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[timescale-automation]

Automated backport to 2.14.x not done: cherry-pick failed.

Git status

HEAD detached at origin/2.14.x
You are currently cherry-picking commit 0d4801988.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   .unreleased/bugfix_6677
	modified:   tsl/src/continuous_aggs/common.h
	modified:   tsl/src/continuous_aggs/invalidation_threshold.c
	modified:   tsl/test/isolation/expected/cagg_concurrent_invalidation.out
	modified:   tsl/test/isolation/specs/cagg_concurrent_invalidation.spec

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   test/runner_isolation.sh
	both modified:   tsl/src/continuous_aggs/insert.c
	both modified:   tsl/test/isolation/expected/cagg_watermark_concurrent_update.out
	both modified:   tsl/test/isolation/expected/cagg_watermark_concurrent_update_1.out

Job log