Skip to content

Use a small table in PutWithExistingIdNoInfiniteLoopWhenFull - #847

Merged
rkennke merged 1 commit into
mainfrom
test/putwithexistingid-small-table
Oct 9, 2026
Merged

rkennke merged 1 commit into
mainfrom
test/putwithexistingid-small-table

Conversation

@rkennke

@rkennke rkennke commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?:

Makes CallTraceStorageTest.PutWithExistingIdNoInfiniteLoopWhenFull seed a 1024-slot table (through the existing CallTraceHashTableTestAccessor) instead of filling the default 65536-slot one. It also checks that inserts beyond the capacity were dropped, i.e. that the full-table path the test is about actually ran.

Motivation:

The test filled 65536 slots, then made 128 more inserts, each probing the whole table before giving up. Under TSan that takes close to the test's fixed 10 s deadline, and it failed intermittently on CI:

Both times it was the only failing test, with no TSan reports, and the same code passed in other pipelines.

The regression it guards, a probe that cycles forever once the table is full, does not depend on the table size. putWithExistingId() never expands a table, so a small seeded table fills up and takes the same hasNext() give-up path.

Additional Notes:

The 10 s deadline stays as the hang detector. The test-only accessor moves above this test so that it is defined before its first use; its content is unchanged.

How to test the change?:

gtestDebug_test_callTraceStorage, gtestAsan_test_callTraceStorage, gtestTsan_test_callTraceStorage: the test now runs in ~52-61 ms in each configuration (previously close to 10 s under TSan).

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a security review (run the dd:platform-security-review
    skill, or file a request via the PSEC review form).
    bewaire also runs automatically on every PR.
  • This PR doesn't touch any of that.
  • JIRA: [PROF-XXXX]

🤖 Generated with Claude Code

The test filled a 65536-slot table and then made 128 more inserts, each
probing the whole table before giving up. Under TSan that took close to
its fixed 10 s deadline and failed intermittently on CI runners (seen in
gtest-tsan-arm64 and gtest-tsan-amd64). The regression it guards, a
probe cycling forever once the table is full, does not depend on the
table size, so seed a 1024-slot table instead. The test now runs in
~60 ms under TSan, keeps the deadline as a hang detector, and checks that
inserts beyond the capacity were dropped, i.e. that the full-table path
actually ran.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rkennke
rkennke requested a review from a team as a code owner October 9, 2026 07:07
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-09T07:10:17.223850Z 552c5f5 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@datadog-datadog-prod-us1 datadog-datadog-prod-us1 Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bits Code Review: PASS

More details

The 1,024-slot table preserves the full-table probe exhaustion scenario, and the added dropped-insert assertion verifies that the overflow path is exercised.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Bits Code Review · Commit 552c5f5 · @DataDog review to ask questions

@dd-octo-sts

dd-octo-sts Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

CI Test Results

Run: #37897230469 | Commit: c5039de | Duration: 17m 25s (longest job)

✅ All 32 test jobs passed

Status Overview

JDK glibc-aarch64/debug glibc-amd64/debug musl-aarch64/debug musl-amd64/debug
8 - ✅ - -
8-ibm - ✅ - -
8-j9 ✅ ✅ - -
8-librca - - ✅ ✅
8-orcl - ✅ - -
11 - ✅ - -
11-j9 ✅ ✅ - -
11-librca - - ✅ ✅
17 ✅ ✅ - -
17-graal ✅ ✅ - -
17-j9 ✅ ✅ - -
17-librca - - ✅ ✅
21 ✅ ✅ - -
21-graal ✅ ✅ - -
21-librca - - ✅ ✅
25 ✅ ✅ - -
25-graal ✅ ✅ - -
25-librca - - ✅ ✅

Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled

Summary: Total: 32 | Passed: 32 | Failed: 0


Updated: 2026-10-09 07:27:05 UTC

@dd-octo-sts

dd-octo-sts Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

✅ All 40 integration tests passed

📊 Dashboard · 👷 Pipeline · 📦 552c5f5f

@rkennke
rkennke merged commit 96a7360 into main Oct 9, 2026
117 checks passed
@rkennke
rkennke deleted the test/putwithexistingid-small-table branch October 9, 2026 09:22
@github-actions github-actions Bot added this to the 1.52.0 milestone Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant