Offline storage: Flush() persists EventPersistence_DoNotStoreOnDisk records to disk when the RAM tier is active

未关闭
#1,504 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

还没有人认领这个 Issue。

评估

难度
5/5
预计耗时
一周以上
新手友好度
35/100
Issue 类型
缺陷
描述清晰度
需要澄清
活跃度
冷清
技术栈
cpp
领域
databases

调研方向

首先阅读 OfflineStorageHandler::Flush()、StoreRecord() 以及此处描述的 enum EventPersistence_DoNotStoreOnDisk 行为。在进行任何更改之前,先与维护者确认预期的契约。完成的标准是:要么记录或重命名“仅磁盘”语义,要么在 flush 期间通过回归测试强制执行“无磁盘”契约。

由索引模型根据 Issue 内容生成。

描述

Summary

OfflineStorageHandler::Flush() drains the entire in-memory queue and persists it to the disk backend in one batch:

auto records = m_offlineStorageMemory->GetRecords(false, EventLatency_Unspecified);
size_t totalSaved = m_offlineStorageDisk->StoreRecords(records);

This batch includes records whose persistence == EventPersistence::EventPersistence_DoNotStoreOnDisk. As a result, when the RAM tier is active (the normal configuration), a DoNotStoreOnDisk record that entered the in-memory queue can be written to disk on a memory-pressure flush and survive a process restart.

By contrast, OfflineStorageHandler::StoreRecord() explicitly skips such records — but only in the disk-only fallback branch (no RAM tier / shutdown):

else // m_offlineStorageMemory == nullptr || m_shutdownStarted
{
    if (m_offlineStorageDisk != nullptr)
    {
        if (record.persistence != EventPersistence::EventPersistence_DoNotStoreOnDisk)
        {
            return m_offlineStorageDisk->StoreRecord(record);
        }
    }
}

When the RAM tier is present, StoreRecord() inserts every record into memory unconditionally (no persistence check), so DoNotStoreOnDisk records ride the normal RAM→disk flush path.

Pre-existing

This behavior is not introduced by the batched-flush change in #1491 — the pre-#1491 Flush() on main already called m_offlineStorageDisk->StoreRecords(records) on the full drained batch with no DoNotStoreOnDisk filter. This issue tracks the semantics question separately so #1491 stays focused.

The open question

The DoNotStoreOnDisk filter existing only in the disk-only fallback branch suggests the current flush-to-disk behavior may be intentional (i.e. the flag means "skip only when forced straight to disk without a RAM tier"). But the enum name implies these records should never be written to disk under any configuration. The two readings disagree.

Options for maintainers

  1. Treat current behavior as intentional and document/rename the enum to make the "disk-only fallback path" semantics explicit, so the name no longer implies a global no-disk contract.
  2. Enforce a global no-disk contract: during flush, partition DoNotStoreOnDisk records out of the disk batch and re-insert them into the RAM queue (rather than draining + persisting them). This keeps them in memory until upload or drop, at the cost of possible RAM accumulation if they are never uploaded, and would need a regression test.

Surfaced during the #1491 review (Copilot). Filing as a follow-up per maintainer direction.

主要语言
C
星标
102
派生
66
平均合并
3 天 10 小时
30 天内合并 PR
10

贡献指南

这个仓库没有索引到贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

microsoft/cpp_client_telemetry 的其他 Issue

查看 microsoft/cpp_client_telemetry 的全部 Issue

相似的 Issue

更多 C Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。