Skip to content

Commit 42dee6c

Browse files
dbeckdbeck
committed
Review comments, eviction and minor simplification
1 parent bc4e1d2 commit 42dee6c

11 files changed

Lines changed: 171 additions & 26 deletions

‎src/chunk.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include <storage/lmgr.h>
4141
#include <storage/lockdefs.h>
4242
#include <tcop/tcopprot.h>
43+
#include <ts_stats/ts_stats_record.h>
4344
#include <utils/acl.h>
4445
#include <utils/array.h>
4546
#include <utils/builtins.h>
@@ -4073,6 +4074,9 @@ ts_chunk_drop(const Chunk *chunk, DropBehavior behavior, int32 log_level)
40734074

40744075
/* Drop the table */
40754076
performDeletion(&objaddr, behavior, 0);
4077+
4078+
/* Evict the chunk stats from the shared memory */
4079+
ts_stats_chunk_evict(chunk->table_id);
40764080
}
40774081

40784082
static void

‎src/guc.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1469,8 +1469,7 @@ _guc_init(void)
14691469
"Per-database statistics cache capacity, "
14701470
"in chunks. 0 disables the feature.",
14711471
"Must be 0 or a power of 2. "
1472-
"Takes effect only for databases whose observability segment "
1473-
"has not yet been created.",
1472+
"Takes effect only after server restart.",
14741473
&ts_guc_stats_max_chunks,
14751474
TS_STATS_MAX_CHUNKS_DEFAULT, /* default */
14761475
0, /* min: 0 = disabled */

‎src/ts_stats/ts_stats_record.c‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@
55
*/
66

77
#include "ts_stats_record.h"
8+
#include "ts_stats/ts_stats_defs.h"
89
#include "ts_stats_segment.h"
910

1011
#include <postgres.h>
1112
#include <miscadmin.h>
1213
#include <storage/lwlock.h>
14+
#include <utils/dsa.h>
1315
#include <utils/timestamp.h>
1416

1517
static inline uint64
@@ -290,3 +292,52 @@ ts_stats_chunk_lookup(Oid compressed_relid, TsStatsChunk *out)
290292
memcpy(out, slot, sizeof(TsStatsChunk));
291293
return true;
292294
}
295+
296+
void
297+
ts_stats_chunk_evict(Oid compressed_relid)
298+
{
299+
if (!OidIsValid(compressed_relid))
300+
{
301+
return;
302+
}
303+
TsStatsChunkSegment *seg = ts_get_stats_chunk_segment();
304+
if (seg == NULL)
305+
{
306+
return;
307+
}
308+
309+
int32 slot_idx = ts_stats_chunk_segment_lookup(seg, compressed_relid);
310+
311+
if (slot_idx == TS_STATS_INVALID_IDX)
312+
{
313+
return;
314+
}
315+
Assert(slot_idx >= 0 && slot_idx < (int32) seg->num_slots);
316+
317+
TsStatsChunkMetadata *meta_base = ts_stats_chunk_metadata(seg);
318+
TsStatsChunkMetadata *meta = &meta_base[slot_idx];
319+
320+
/* limited CAS loop to ensure we don't evict a chunk that is currently being updated, the danger
321+
* is that the one that we are evicting is already being overwritten by a newer/other chunk.
322+
*/
323+
uint64 state = pg_atomic_read_u64(&meta->state);
324+
uint64 new_seqno = pg_atomic_add_fetch_u64(&seg->update_seqno, 1);
325+
uint64 new_state = (new_seqno << TS_STATS_CHUNK_METADATA_SEQNO_SHIFT);
326+
for (int i = 0;
327+
i < 5 &&
328+
/* the slot still matches the compressed_relid */
329+
meta->compressed_relid == compressed_relid &&
330+
/* the slot is still valid and not in progress */
331+
TS_STATS_CHUNK_METADATA_IS_VALID(state) &&
332+
!(TS_STATS_CHUNK_METADATA_IS_IN_PROGRESS(state)) &&
333+
/* the slot being updated is older than the sequence number of the current update */
334+
TS_STATS_CHUNK_METADATA_GET_SEQNO(state) < new_seqno;
335+
++i)
336+
{
337+
if (pg_atomic_compare_exchange_u64(&meta->state, &state, new_state))
338+
{
339+
meta->compressed_relid = InvalidOid;
340+
break;
341+
}
342+
}
343+
}

‎src/ts_stats/ts_stats_record.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,7 @@ extern TSDLLEXPORT void ts_stats_chunk_record_decompression(Oid compressed_relid
7474
/* Lookup TsStatsChunk based on the compressed relid. It returns true if the chunk is found, false
7575
* otherwise. */
7676
extern TSDLLEXPORT bool ts_stats_chunk_lookup(Oid compressed_relid, TsStatsChunk *out);
77+
78+
/* Evict the chunk information for a given compressed relid. This function is called when we
79+
* drop a compressed chunk. */
80+
extern TSDLLEXPORT void ts_stats_chunk_evict(Oid compressed_relid);

‎src/ts_stats/ts_stats_segment.c‎

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -385,14 +385,16 @@ get_sorted_bucket_in_desc_order(TsStatsChunkSegment *seg, uint8 *out)
385385
*/
386386
static inline int32
387387
ts_stats_chunk_update_at(TsStatsChunkSegment *seg, Oid compressed_relid, Oid uncompressed_relid,
388-
int32 idx, uint8 bucket_idx, uint64 old_state)
388+
int32 idx, uint64 old_state)
389389
{
390390
Assert(seg != NULL || OidIsValid(compressed_relid) || OidIsValid(uncompressed_relid));
391391
if (seg == NULL || !OidIsValid(compressed_relid) || !OidIsValid(uncompressed_relid))
392392
{
393393
return TS_STATS_INVALID_IDX;
394394
}
395395

396+
uint32 bucket_size = seg->num_slots / TS_STATS_BUCKETS;
397+
uint32 bucket_idx = idx / bucket_size;
396398
uint64 new_seqno = pg_atomic_add_fetch_u64(&seg->update_seqno, 1);
397399
uint64 new_state = (new_seqno << TS_STATS_CHUNK_METADATA_SEQNO_SHIFT) |
398400
TS_STATS_CHUNK_METADATA_VALID_FLAG |
@@ -475,7 +477,6 @@ ts_stats_chunk_segment_prepare_upsert(TsStatsChunkSegment *seg, Oid compressed_r
475477
compressed_relid,
476478
uncompressed_relid,
477479
idx,
478-
bucket_idx,
479480
state) != TS_STATS_INVALID_IDX)
480481
{
481482
return (int32) idx;
@@ -493,7 +494,6 @@ ts_stats_chunk_segment_prepare_upsert(TsStatsChunkSegment *seg, Oid compressed_r
493494
compressed_relid,
494495
uncompressed_relid,
495496
idx,
496-
bucket_idx,
497497
state) != TS_STATS_INVALID_IDX)
498498
{
499499
return (int32) idx;
@@ -525,17 +525,11 @@ ts_stats_chunk_segment_prepare_upsert(TsStatsChunkSegment *seg, Oid compressed_r
525525
continue;
526526
}
527527

528-
uint8 bucket_idx = idx / bucket_size;
529-
530528
/* try to claim this slot, and it will update its state to in-progress
531529
* and the bucket's last update sequence number.
532530
*/
533-
if (ts_stats_chunk_update_at(seg,
534-
compressed_relid,
535-
uncompressed_relid,
536-
idx,
537-
bucket_idx,
538-
state) != TS_STATS_INVALID_IDX)
531+
if (ts_stats_chunk_update_at(seg, compressed_relid, uncompressed_relid, idx, state) !=
532+
TS_STATS_INVALID_IDX)
539533
{
540534
TsStatsChunk *slot = &slot_base[idx];
541535
ts_stats_chunk_init_slot(slot);
@@ -573,12 +567,8 @@ ts_stats_chunk_segment_prepare_upsert(TsStatsChunkSegment *seg, Oid compressed_r
573567
{
574568
/* this slot is empty, although at this point it should not happen
575569
* as the previous loops should have handled empty slots. */
576-
if (ts_stats_chunk_update_at(seg,
577-
compressed_relid,
578-
uncompressed_relid,
579-
idx,
580-
oldest_bucket_idx,
581-
state) != TS_STATS_INVALID_IDX)
570+
if (ts_stats_chunk_update_at(seg, compressed_relid, uncompressed_relid, idx, state) !=
571+
TS_STATS_INVALID_IDX)
582572
{
583573
TsStatsChunk *slot = &slot_base[idx];
584574
ts_stats_chunk_init_slot(slot);
@@ -610,7 +600,6 @@ ts_stats_chunk_segment_prepare_upsert(TsStatsChunkSegment *seg, Oid compressed_r
610600
compressed_relid,
611601
uncompressed_relid,
612602
candidate_idx,
613-
oldest_bucket_idx,
614603
state);
615604

616605
if (updated_idx != TS_STATS_INVALID_IDX)

‎src/ts_stats/ts_stats_srf.c‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,12 +100,6 @@ ts_stats_chunks(PG_FUNCTION_ARGS)
100100
TsStatsChunk *slot_base = ts_stats_chunk_slots(seg);
101101
TsStatsChunkMetadata *meta_base = ts_stats_chunk_metadata(seg);
102102

103-
Assert(slot_base != NULL && meta_base != NULL);
104-
if (slot_base == NULL || meta_base == NULL)
105-
{
106-
PG_RETURN_VOID();
107-
}
108-
109103
if (comp_filter)
110104
{
111105
/* Point lookup based on compressed_relid */

‎tsl/test/expected/compress_observ-15.out‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,28 @@ compressed_block_bytes_max | 676
867867
compressed_block_bytes_avg | 482
868868
compressed_block_bytes_stddev | 150.983442800858
869869

870+
-- The stats should be evicted after we decompress and thus throwing away
871+
-- the compressed chunks.
872+
SELECT decompress_chunk(c) FROM show_chunks('t') c;
873+
-[ RECORD 1 ]----+---------------------------------------
874+
decompress_chunk | _timescaledb_internal._hyper_1_1_chunk
875+
-[ RECORD 2 ]----+---------------------------------------
876+
decompress_chunk | _timescaledb_internal._hyper_1_2_chunk
877+
-[ RECORD 3 ]----+---------------------------------------
878+
decompress_chunk | _timescaledb_internal._hyper_1_3_chunk
879+
880+
-- This should be empty after the decompression
881+
SELECT
882+
chunk, compressed_chunk,
883+
compressed_batch_count, compressed_block_count,
884+
compressed_batch_rows_min, compressed_batch_rows_max,
885+
compressed_batch_rows_avg, compressed_batch_rows_stddev,
886+
compressed_batch_bytes_min, compressed_batch_bytes_max,
887+
compressed_batch_bytes_avg, compressed_batch_bytes_stddev,
888+
compressed_block_bytes_min, compressed_block_bytes_max,
889+
compressed_block_bytes_avg, compressed_block_bytes_stddev
890+
FROM observ_main_view;
891+
870892
DROP VIEW observ;
871893
DROP VIEW observ_main_view;
872894
DROP TABLE t;

‎tsl/test/expected/compress_observ-16.out‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,28 @@ compressed_block_bytes_max | 676
867867
compressed_block_bytes_avg | 482
868868
compressed_block_bytes_stddev | 150.983442800858
869869

870+
-- The stats should be evicted after we decompress and thus throwing away
871+
-- the compressed chunks.
872+
SELECT decompress_chunk(c) FROM show_chunks('t') c;
873+
-[ RECORD 1 ]----+---------------------------------------
874+
decompress_chunk | _timescaledb_internal._hyper_1_1_chunk
875+
-[ RECORD 2 ]----+---------------------------------------
876+
decompress_chunk | _timescaledb_internal._hyper_1_2_chunk
877+
-[ RECORD 3 ]----+---------------------------------------
878+
decompress_chunk | _timescaledb_internal._hyper_1_3_chunk
879+
880+
-- This should be empty after the decompression
881+
SELECT
882+
chunk, compressed_chunk,
883+
compressed_batch_count, compressed_block_count,
884+
compressed_batch_rows_min, compressed_batch_rows_max,
885+
compressed_batch_rows_avg, compressed_batch_rows_stddev,
886+
compressed_batch_bytes_min, compressed_batch_bytes_max,
887+
compressed_batch_bytes_avg, compressed_batch_bytes_stddev,
888+
compressed_block_bytes_min, compressed_block_bytes_max,
889+
compressed_block_bytes_avg, compressed_block_bytes_stddev
890+
FROM observ_main_view;
891+
870892
DROP VIEW observ;
871893
DROP VIEW observ_main_view;
872894
DROP TABLE t;

‎tsl/test/expected/compress_observ-17.out‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,28 @@ compressed_block_bytes_max | 676
867867
compressed_block_bytes_avg | 482
868868
compressed_block_bytes_stddev | 150.983442800858
869869

870+
-- The stats should be evicted after we decompress and thus throwing away
871+
-- the compressed chunks.
872+
SELECT decompress_chunk(c) FROM show_chunks('t') c;
873+
-[ RECORD 1 ]----+---------------------------------------
874+
decompress_chunk | _timescaledb_internal._hyper_1_1_chunk
875+
-[ RECORD 2 ]----+---------------------------------------
876+
decompress_chunk | _timescaledb_internal._hyper_1_2_chunk
877+
-[ RECORD 3 ]----+---------------------------------------
878+
decompress_chunk | _timescaledb_internal._hyper_1_3_chunk
879+
880+
-- This should be empty after the decompression
881+
SELECT
882+
chunk, compressed_chunk,
883+
compressed_batch_count, compressed_block_count,
884+
compressed_batch_rows_min, compressed_batch_rows_max,
885+
compressed_batch_rows_avg, compressed_batch_rows_stddev,
886+
compressed_batch_bytes_min, compressed_batch_bytes_max,
887+
compressed_batch_bytes_avg, compressed_batch_bytes_stddev,
888+
compressed_block_bytes_min, compressed_block_bytes_max,
889+
compressed_block_bytes_avg, compressed_block_bytes_stddev
890+
FROM observ_main_view;
891+
870892
DROP VIEW observ;
871893
DROP VIEW observ_main_view;
872894
DROP TABLE t;

‎tsl/test/expected/compress_observ-18.out‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,28 @@ compressed_block_bytes_max | 668
867867
compressed_block_bytes_avg | 460
868868
compressed_block_bytes_stddev | 150.412765415705
869869

870+
-- The stats should be evicted after we decompress and thus throwing away
871+
-- the compressed chunks.
872+
SELECT decompress_chunk(c) FROM show_chunks('t') c;
873+
-[ RECORD 1 ]----+---------------------------------------
874+
decompress_chunk | _timescaledb_internal._hyper_1_1_chunk
875+
-[ RECORD 2 ]----+---------------------------------------
876+
decompress_chunk | _timescaledb_internal._hyper_1_2_chunk
877+
-[ RECORD 3 ]----+---------------------------------------
878+
decompress_chunk | _timescaledb_internal._hyper_1_3_chunk
879+
880+
-- This should be empty after the decompression
881+
SELECT
882+
chunk, compressed_chunk,
883+
compressed_batch_count, compressed_block_count,
884+
compressed_batch_rows_min, compressed_batch_rows_max,
885+
compressed_batch_rows_avg, compressed_batch_rows_stddev,
886+
compressed_batch_bytes_min, compressed_batch_bytes_max,
887+
compressed_batch_bytes_avg, compressed_batch_bytes_stddev,
888+
compressed_block_bytes_min, compressed_block_bytes_max,
889+
compressed_block_bytes_avg, compressed_block_bytes_stddev
890+
FROM observ_main_view;
891+
870892
DROP VIEW observ;
871893
DROP VIEW observ_main_view;
872894
DROP TABLE t;

0 commit comments

Comments
 (0)