Skip to content

Commit 6d73137

Browse files
committed
Merge branch 'dev' of https://github.com/yayen-lin/sirius into sirius-array-type
2 parents 5692747 + 81beb83 commit 6d73137

152 files changed

Lines changed: 6303 additions & 4971 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.claude/skills/log-analyzer/SKILL.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,4 +198,33 @@ After you've localized the issue, recommend the right next step:
198198
| Re-materialized inputs after a downgrade (per-task view) | `<folder>/memory_history.csv -> peak_bytes_to_materialize_input > 0` |
199199
| Per-pipeline summary across one query | filter `_pipeline_aggregates.csv` by `query_begin_ts` |
200200
| How well work was balanced across GPUs (multi-GPU runs) | group `task_outputs.csv` by `gpu_id` (per pipeline or per operator type); see `references/single_query.md` § "GPU balance" |
201+
| Host (pinned) pool memory at query start/end | `<folder>/query_meta.json -> pool_stats.host` |
202+
| GPU device memory pool at query start/end (per GPU) | `<folder>/query_meta.json -> pool_stats.gpu[]` |
201203
| Why my parser run looks weird | `_summary.json -> format_warnings`, then `tools/log_analyzer/README.md` |
204+
205+
## Pool stats: detecting memory leaks across queries
206+
207+
`query_meta.json` contains a `pool_stats` section that captures memory state at the query boundary:
208+
209+
```json
210+
{
211+
"pool_stats": {
212+
"host": {
213+
"begin": {"allocated_bytes": 5242880, "peak_bytes": 307232768, "free_blocks": 5115},
214+
"end": {"allocated_bytes": 5242880, "peak_bytes": 307232768, "free_blocks": 5115}
215+
},
216+
"gpu": [
217+
{
218+
"device_id": 0,
219+
"begin": {"allocated_bytes": 0, "peak_bytes": 104857600, "reserved_bytes": 107374182400},
220+
"end": {"allocated_bytes": 0, "peak_bytes": 104857600, "reserved_bytes": 107374182400}
221+
}
222+
]
223+
}
224+
}
225+
```
226+
227+
- **Host pool** (`[query_pool]` log tag): uses a `fixed_size_host_memory_resource` slab allocator. `free_blocks` drops during the query and should return to the QueryBegin value at QueryEnd. A persistent `end.allocated_bytes > begin.allocated_bytes` is a pinned-host memory leak.
228+
- **GPU pool** (`[gpu_pool]` log tag, one line per GPU): uses a `reservation_aware_resource_adaptor`. `allocated_bytes` reflects live GPU allocations across all streams; `reserved_bytes` reflects the capacity claimed by active reservations. A persistent `end.allocated_bytes > begin.allocated_bytes` is a GPU device memory leak.
229+
230+
The two pool tags are distinguishable in the raw log: host stats use `[query_pool]`, GPU stats use `[gpu_pool]`.

.github/workflows/check.yml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,24 @@ jobs:
146146
env:
147147
CUDAARCHS: "100"
148148

149+
# Rust bindings (sirius-sys + sirius) — fmt/clippy + build/link the test.
150+
# Run on a single representative entry. `cargo test --no-run` compiles the
151+
# cxx glue against the Sirius FFI header and links the proof-of-life test
152+
# against the single Sirius library built above (build.rs discovers it under
153+
# build/release). It is NOT run here: constructing a context does GPU
154+
# bring-up, and these are CPU runners — the actual run belongs on a GPU
155+
# runner (test workflow).
156+
- name: Rust bindings checks
157+
if: >-
158+
matrix.build.arch == 'x64' && matrix.build.compiler == 'gcc'
159+
&& matrix.build.build-type == 'release' && matrix.build.environment == 'default'
160+
&& matrix.build.cudf-channel == 'stable' && matrix.build.variant == 'super'
161+
run: |
162+
set -euo pipefail
163+
pixi run cargo fmt --manifest-path rust/Cargo.toml --package sirius --package sirius-sys --check
164+
pixi run cargo clippy --manifest-path rust/Cargo.toml --package sirius --package sirius-sys --all-targets -- -D warnings
165+
pixi run cargo test --no-run --manifest-path rust/Cargo.toml --package sirius --package sirius-sys
166+
149167
- name: Build (vcpkg)
150168
if: matrix.build.environment == 'vcpkg'
151169
run: |

.github/workflows/experimental.yml

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
# NOTICE - Actions permissions policy (Settings > Actions > General):
2+
# - "Allow sirius-db, and select non-sirius-db, actions and reusable workflows"
3+
# - Allow actions created by GitHub: YES (covers actions/*)
4+
# - Allow actions by Marketplace verified creators: NO
5+
# - Explicitly allowed third-party actions:
6+
# prefix-dev/setup-pixi@*
7+
# mozilla-actions/sccache-action@*
8+
# - Require actions to be pinned to a full-length commit SHA: NO
9+
#
10+
# Adding a new action? You must:
11+
# 1. Pin it to a full-length commit SHA (e.g. uses: owner/repo@<40-char-sha> # vX.Y.Z)
12+
# 2. If it is not from sirius-db, GitHub, or the explicitly allowed third-party actions above, add it to the
13+
# project-level allowlist above before merging
14+
name: Experimental
15+
16+
on:
17+
pull_request:
18+
paths:
19+
- 'experimental/**'
20+
- '.github/workflows/experimental.yml'
21+
merge_group:
22+
23+
concurrency:
24+
group: ${{ github.workflow }}-${{ github.ref }}
25+
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
26+
27+
jobs:
28+
starrocks:
29+
name: starrocks
30+
runs-on: ubuntu-24.04
31+
defaults:
32+
run:
33+
working-directory: experimental/starrocks
34+
steps:
35+
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
36+
with:
37+
submodules: false
38+
39+
- name: Init starrocks submodule
40+
working-directory: ${{ github.workspace }}
41+
run: git submodule update --init --depth=1 experimental/starrocks/starrocks
42+
43+
- uses: prefix-dev/setup-pixi@5185adfbffb4bd703da3010310260805d89ebb11 # v0.9.6
44+
with:
45+
cache: false
46+
pixi-version: v0.70.2
47+
manifest-path: experimental/starrocks/pixi.toml
48+
environments: cn
49+
50+
- name: fmt
51+
run: pixi run -e cn cargo fmt --all -- --check
52+
53+
- name: clippy
54+
run: pixi run -e cn cargo clippy --all-targets -- -D warnings
55+
56+
- name: test
57+
run: pixi run -e cn cargo test --workspace
58+
59+
experimental:
60+
name: experimental
61+
runs-on: ubuntu-24.04
62+
needs: [starrocks]
63+
if: always()
64+
steps:
65+
- name: All checks ok
66+
if: ${{ needs.starrocks.result == 'success' }}
67+
run: exit 0
68+
- name: Some checks failed
69+
if: ${{ needs.starrocks.result != 'success' }}
70+
run: exit 1

.gitignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ log/
4848

4949
.planning/
5050

51-
# Generated S3 integration-test fixtures (regenerated by test/cpp/integration/s3/fixtures.sh)
51+
# Generated S3 integration-test fixtures (regenerated by generate_fixtures.py;
52+
# at test time the harness writes them under a temp dir instead)
5253
/test/cpp/integration/s3/fixtures/local/
5354
/test/cpp/integration/s3/fixtures/generated/
5455
/test/cpp/integration/s3/certs/

.pre-commit-config.yaml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313
# See https://github.com/pre-commit/pre-commit
1414

1515
# Global exclusions, csv are excluded because they are often test answers, and experimental is out of lint scope.
16-
exclude: \.csv$|^legacy/queries/.*|^experimental/.*
16+
# .patch files are verbatim diffs: formatters (trailing-whitespace, end-of-file-fixer)
17+
# strip the leading context space on blank lines and corrupt `git apply`.
18+
exclude: \.csv$|\.patch$|^legacy/queries/.*|^experimental/.*
1719

1820
repos:
1921
# Standard hooks

CMakeLists.txt

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,14 @@ endif()
2828
option(ENABLE_STREAM_CHECK
2929
"Enable stream check library for debugging default stream usage" OFF)
3030

31+
# The [s3] integration tests launch MinIO via a patched build of upstream
32+
# testcontainers-native, fetched + patched at configure time and built as a Go
33+
# c-archive — so it needs a Go toolchain and network at configure/build time. ON
34+
# by default (the pixi env provides Go); turn OFF for offline / Go-less /
35+
# release / package builds, which then skip the S3 container harness entirely.
36+
option(SIRIUS_BUILD_S3_TESTS
37+
"Build the testcontainers-backed S3 integration test harness" ON)
38+
3139
# In the vcpkg build, vcpkg headers must take priority over: 1) conda's CCCL
3240
# 3.1.4 (injected via CXXFLAGS as -I.../include/cccl) 2) DuckDB's bundled fmt
3341
# 6.1.2 (uses duckdb_fmt namespace, incompatible with spdlog) Fix: strip conda
@@ -139,6 +147,15 @@ endif()
139147
# Rust telemetry instrumentation (C++ FFI via Corrosion)
140148
add_subdirectory(rust/crates/telemetry/bridge)
141149

150+
# Upstream testcontainers-native, fetched + patched at configure time (see
151+
# cmake/testcontainers_native.cmake), used by the S3 integration test harness to
152+
# start MinIO containers from the test binary. Builds a Go c-archive, so a Go
153+
# toolchain (provided by pixi) and network access on the first configure/build
154+
# are required — hence gated behind SIRIUS_BUILD_S3_TESTS.
155+
if(SIRIUS_BUILD_S3_TESTS)
156+
include("${CMAKE_CURRENT_SOURCE_DIR}/cmake/testcontainers_native.cmake")
157+
endif()
158+
142159
# cmake-format: off
143160
set(EXTENSION_SOURCES
144161
src/config.cpp
@@ -150,8 +167,8 @@ set(EXTENSION_SOURCES
150167
src/data/host_parquet_representation_converters.cpp
151168
src/downgrade/downgrade_executor.cpp
152169
src/expression/aggregate_id.cpp
153-
src/expression/ast/clone.cpp
154170
src/expression/ast/node.cpp
171+
src/expression/ast/utils.cpp
155172
src/expression/from_duckdb.cpp
156173
src/expression/function_id.cpp
157174
src/expression/join_condition.cpp
@@ -196,20 +213,15 @@ set(EXTENSION_SOURCES
196213
src/op/partition/gpu_partition_impl.cpp
197214
src/op/result/host_table_chunk_reader.cpp
198215
src/op/scan/cached_ranges.cpp
199-
src/op/scan/duckdb_scan_executor.cpp
200-
src/op/scan/cpu_source_task.cpp
201216
src/op/scan/duckdb_native_metadata.cpp
202217
src/op/scan/duckdb_native_decoder.cpp
203-
src/op/scan/duckdb_scan_task.cpp
204218
src/op/scan/equality_delete_filter.cpp
205219
src/op/scan/hive_partition.cpp
206220
src/op/scan/iceberg_avro_reader.cpp
207221
src/op/scan/iceberg_delete_pipeline.cpp
208222
src/op/scan/iceberg_metadata_reader.cpp
209-
src/op/scan/iceberg_scan_task.cpp
210223
src/op/scan/positional_delete_filter.cpp
211224
src/op/scan/puffin_reader.cpp
212-
src/op/scan/parquet_scan_task.cpp
213225
src/op/scan/parquet_schema_mapping.cpp
214226
src/op/scan/prefetched_data_source.cpp
215227
src/op/scan/scan_plan.cpp
@@ -275,17 +287,20 @@ set(EXTENSION_SOURCES
275287
src/planner/sirius_plan_limit.cpp
276288
src/planner/sirius_plan_order.cpp
277289
src/planner/sirius_plan_projection.cpp
290+
src/planner/sirius_plan_projection_utils.cpp
278291
src/planner/sirius_plan_recursive_cte.cpp
279292
src/planner/sirius_plan_top_n.cpp
280293
src/pin_table.cpp
281294
src/scan_manager/gpu_ingestible_factory.cpp
295+
src/scan_manager/round_robin_strategy.cpp
282296
src/scan_manager/sirius_scan_manager.cpp
283297
src/scan_manager/split_connector.cpp
284298
src/scan_manager/split_provider.cpp
285299
src/sirius_config.cpp
286300
src/sirius_context.cpp
287301
src/sirius_engine.cpp
288302
src/sirius_extension.cpp
303+
src/sirius_ffi.cpp
289304
src/sirius_interface.cpp
290305
src/sirius_sql_rewrite.cpp
291306
src/telemetry/telemetry_context.cpp
@@ -372,6 +387,11 @@ target_link_libraries(
372387
target_link_libraries(sirius_loadable_extension PkgConfig::LIBURING
373388
PkgConfig::CURL OpenSSL::Crypto)
374389

390+
# The sirius-sys + sirius Rust crates are built by cargo, not CMake (unlike the
391+
# telemetry bridge above, which CMake drives via Corrosion). Their build.rs
392+
# discovers the Sirius headers (repo + conda) and links the libsirius artifact
393+
# this build produces under build/<preset>/; see rust/crates/sirius-sys.
394+
375395
# cucascade upstream PRs #126/#128/#130 moved cucascade to static-CUDA linkage
376396
# and pulled the NVML *static stub* (libnvidia-ml.a) into libcucascade.a via the
377397
# `CUDA::nvml_static` imported target. Upstream's
@@ -445,6 +465,7 @@ set(TEST_SOURCES
445465
test/cpp/exec/test_interruptible_mpmc.cpp
446466
test/cpp/expression/test_ast_aggregate.cpp
447467
test/cpp/expression/test_ast_clone.cpp
468+
test/cpp/expression/test_ast_substitute.cpp
448469
test/cpp/expression/test_ast_from_duckdb.cpp
449470
test/cpp/expression/test_ast_scaffold.cpp
450471
test/cpp/expression/test_ast_to_duckdb.cpp
@@ -461,11 +482,9 @@ set(TEST_SOURCES
461482
test/cpp/integration/test_gpu_execution_array.cpp
462483
test/cpp/integration/test_gpu_execution_locality.cpp
463484
test/cpp/integration/test_gpu_execution_multi_format.cpp
464-
test/cpp/integration/test_gpu_execution_order_nulls.cpp
465485
test/cpp/integration/test_gpu_execution_tpch.cpp
466486
test/cpp/integration/test_gpu_execution_tpch_mgpu_audit.cpp
467487
test/cpp/integration/test_table_gpu_cache_warm_mgpu.cpp
468-
test/cpp/integration/test_transparent_execution.cpp
469488
test/cpp/io/s3/test_sigv4.cpp
470489
test/cpp/io/s3/test_s3_default_visibility_guard.cpp
471490
test/cpp/io/s3/test_s3_blocking_ioctx.cpp
@@ -484,7 +503,6 @@ set(TEST_SOURCES
484503
test/cpp/operator/test_physical_hash_join_mgpu.cpp
485504
test/cpp/operator/test_physical_order_mgpu.cpp
486505
test/cpp/operator/test_mgpu_stress.cpp
487-
test/cpp/memory/test_host_table_utils.cpp
488506
test/cpp/memory/test_multiple_blocks_allocation_accessor.cpp
489507
test/cpp/operator/aggregate/test_gpu_merge_impl.cpp
490508
test/cpp/operator/aggregate/test_physical_grouped_aggregate.cpp
@@ -509,31 +527,29 @@ set(TEST_SOURCES
509527
test/cpp/operator/test_sirius_dynamic_filter.cpp
510528
test/cpp/parallel/test_task_executor.cpp
511529
test/cpp/planner/test_distinct_hash_join_detection.cpp
530+
test/cpp/planner/test_projection_fold.cpp
512531
test/cpp/pipeline/test_gpu_pipeline_executor.cpp
513532
test/cpp/pipeline/test_oom_reschedule.cpp
514533
test/cpp/pipeline/test_pipeline_memory_history.cpp
515534
test/cpp/pipeline/test_gpu_pipeline_task_history.cpp
516535
test/cpp/pipeline/test_gpu_pipeline_disk_readback.cpp
517536
test/cpp/pipeline/test_get_next_ports_after_sink.cpp
518-
test/cpp/pipeline/test_plan_printer.cpp
519537
test/cpp/pipeline/test_repository_wiring_materializer.cpp
520538
test/cpp/pipeline/test_task_scheduler.cpp
521539
test/cpp/scan/bench_decode_codecs.cpp
522-
test/cpp/scan/test_column_builder.cpp
523540
test/cpp/scan/test_duckdb_block_layout.cpp
541+
test/cpp/scan/test_duckdb_native_batch_coalescer.cpp
542+
test/cpp/scan/test_duckdb_native_gpu_ingestible.cpp
524543
test/cpp/scan/test_duckdb_native_walker.cpp
525544
test/cpp/scan/test_gpu_decode_alp.cpp
526545
test/cpp/scan/test_gpu_decode_bitpacking.cpp
527546
test/cpp/scan/test_gpu_decode_rle.cpp
528547
test/cpp/scan/test_gpu_decode_strings.cpp
529548
test/cpp/scan/test_gpu_native_decode.cpp
530549
test/cpp/scan/test_parquet_schema_mapping.cpp
531-
test/cpp/scan/test_parquet_scan_task.cpp
532550
test/cpp/scan/test_parquet_gpu_ingestible.cpp
533551
test/cpp/scan/test_pinned_table_gpu_ingestible.cpp
534-
test/cpp/scan/test_scan_executor.cpp
535552
test/cpp/scan/test_split_connector.cpp
536-
test/cpp/scan_manager/test_sirius_scan_manager_s3.cpp
537553
test/cpp/scan_manager/test_describe_parquet_s3.cpp
538554
test/cpp/scan_manager/test_pin_table_multi_gpu.cpp
539555
test/cpp/sirius_extension/test_sirius_read_parquet_cardinality.cpp
@@ -547,6 +563,12 @@ if(SIRIUS_LEGACY_TEST_SOURCES)
547563
list(APPEND TEST_SOURCES ${SIRIUS_LEGACY_TEST_SOURCES})
548564
endif()
549565

566+
# The testcontainers-backed S3 harness only compiles when its dependency is
567+
# built; unittest.cpp guards its use behind SIRIUS_HAVE_TESTCONTAINERS.
568+
if(SIRIUS_BUILD_S3_TESTS)
569+
list(APPEND TEST_SOURCES test/cpp/utils/s3_container.cpp)
570+
endif()
571+
550572
add_executable(sirius_unittest ${TEST_SOURCES})
551573

552574
# TPC-DS integration test fixture, generated at build time (opt-in target). The
@@ -583,6 +605,16 @@ target_include_directories(
583605

584606
target_link_libraries(sirius_unittest sirius_extension duckdb_static)
585607
link_extension_libraries(sirius_unittest "")
608+
609+
# S3 container harness: the testcontainers-native bridge plus libcurl for
610+
# host-side fixture upload (SigV4 signing comes from sirius_extension). Gated so
611+
# offline/Go-less builds skip it; the harness calls in unittest.cpp are guarded
612+
# by SIRIUS_HAVE_TESTCONTAINERS.
613+
if(SIRIUS_BUILD_S3_TESTS)
614+
target_link_libraries(sirius_unittest testcontainers_native PkgConfig::CURL)
615+
target_compile_definitions(sirius_unittest
616+
PRIVATE SIRIUS_HAVE_TESTCONTAINERS=1)
617+
endif()
586618
# DuckDB v1.5.0 unity builds emit strong symbols for static constexpr members
587619
# that conflict with inline definitions from headers included in test files.
588620
target_link_options(sirius_unittest PRIVATE

0 commit comments

Comments
 (0)