后端

当排序静默地将文件重新绑定到错误的记录时

原地排序加上从索引派生的文件名,会产生可以正常构建、加载和验证的输出,但同时会将每条记录指向错误的文件。

本文由 AI 模型从英文原文翻译而来,措辞可能与原文有出入。 阅读英文原文

某个流水线按组处理项目,然后将它们展平到一个切片中,用于一个共享的转换步骤,并使用循环索引将每个输出文件写入为 item-%04d.ext。然后,有人要求将输出按所有者重新分组。这个显而易见的更改所生成的文件确实存在,大小也正确,通过了每一项模式检查,但其中每一条记录的数据却都是错误的。

没有抛出任何异常。构建通过了,测试通过了,并且构建产物也能在下游工具中以正确的结构打开。只有有效负载是错误的,而有效负载恰恰是自动化检查通常不会去检查的内容。

造成此问题的设置

三个普通的决定组合成了一个陷阱。单独来看,每个决定都是合理的。

首先,一个共享的处理步骤接收一个扁平化的切片,因为一次性批量下载和转换所有条目比按组处理更经济。

其次,该步骤会进行原地排序。当后续工作需要依赖顺序时,按时间线位置、按键或按任何东西排序都是很常见的。排序是正确的,解释排序的注释也是准确的。

第三,输出文件名来自写入时的循环索引。

for i := range items {
    items[i].LocalPath = filepath.Join(dir, fmt.Sprintf("item-%04d.src", i))
}
sortItems(items)                                     // in-place, reorders everything
for i := range items {
    out := filepath.Join(dir, fmt.Sprintf("item-%04d.out", i))
    convert(items[i].LocalPath, out)
    os.Remove(items[i].LocalPath)                    // source deleted, field not updated
}

请仔细阅读。输入按排序前的索引命名。输出按排序后的索引命名。LocalPath 字段永远不会更新为输出,因此在转换后,它指向一个不再存在的文件。记录与其转换后文件之间唯一保留的链接是其在已排序切片中的位置

只要一个循环写入清单,这种方法就有效:

for i, item := range items {
    manifest = append(manifest, Entry{Path: fmt.Sprintf("item-%04d.out", i), ...})
}

切片相同,顺序相同,索引相同。因巧合而正确。

导致问题的变更

现在,按所有者对输出进行分组。自然的实现方式是遍历每个组,并为其文件编号:

idx := 0
for _, group := range groups {
    for _, item := range group.Items {
        group.Entries = append(group.Entries, Entry{
            Path: fmt.Sprintf("item-%04d.out", idx),   // fresh numbering per group
        })
        idx++
    }
}

清单中的每个路径都存在于磁盘上。每个路径都是唯一的。每个文件都具有合理的内容、正确的类型和大致正确的大小。而且,它们中的每一个都属于不同的记录,因为扁平化切片是排序的,而分组遍历不是按排序顺序进行的。

这种失败模式值得专门命名:**该错误不会产生丢失的文件或损坏的文件。它会产生一个内容错误但格式有效的文件。**存在性检查会通过。大小检查会通过。格式验证会通过。模式验证会通过。下游消费者会毫无问题地打开它。

在音频管线中,这意味着剪辑出现在正确的时间线位置,具有正确的时长,但声音是错误的。在文档管线中,这意味着页数正确,但页面内容被调换了。在图像管线中,尺寸正确,但主体是错误的。其症状总是语义上的,而非结构上的,这就是为什么它能通过你所有的结构性检查。

为什么显而易见的修复方法还不够

第一反应是添加一个所有者字段,以便成员关系在排序后得以保留:

type Item struct {
    // ...
    GroupIdx int   // survives the in-place sort
}

这是必要的,而且它确实恢复了分组。但它并没有修复文件绑定,因为该绑定从未依赖于分组。它依赖于位置。添加 GroupIdx 让你能够重建分组,然后在分组内部重新生成位置,而这正是产生交换的代码路径。

我在第一遍处理中犯了这个错误。字段添加进去了,分组也正常工作了,但一位审查者指出,清单构建器仍然根据一个计数器来派生路径。真正有效的修复方法是完全停止派生路径:

// conversion updates the record to point at its own output
for i := start; i < end; i++ {
    os.Remove(items[i].LocalPath)
    items[i].LocalPath = outputs[i-start]     // the record carries its file
}

// manifest reads the field instead of computing an index
for _, item := range items {
    entries[item.GroupIdx] = append(entries[item.GroupIdx], Entry{
        Path: item.LocalPath,
    })
}

现在,记录携带其自身的构建产物引用。重排序、重分组、筛选和并行处理都变得无害,因为没有任何东西会重新计算记录已知的内容。

一般规则是:**当一条记录和其构建产物必须保持在一起时,将引用存储在记录上。**索引是一个位置,而位置是任何排序、筛选或分区操作首先会破坏的东西。一旦两个不同的循环都计算“第 N 个文件”,你就拥有了一个无人强制执行的不变量。

测试这个问题比看起来要难

我针对这个 bug 写了一个回归测试。它设置了两个带有交错排序键的组,运行了流水线阶段,并断言每个条目都指向其自身记录的文件。测试通过了。

当我还原修复时,测试也通过了。

测试在测试文件中重新实现了排序和清单构建,因为生产环境中的函数被埋藏在一个需要网络调用和子进程的更大例程中。测试逻辑的副本只能验证该副本是正确的。它对于要交付的代码毫无说明。

修复方法是提取出纯粹的部分,以便测试可以直接调用它们:

func flattenWithGroupIdx(groups []Group) []Item      // tag membership
func sortItems(items []Item)                          // the actual sort, now callable
func groupIntoEntries(groups []Group, items []Item) []GroupEntries

三次小的提取,无行为变更,并且测试现在会执行交付的代码。然后我验证了当存在该 bug 时,测试确实会失败:

$ # revert manifest to index-derived paths
$ go test -run TestKeepsBinding ./...
--- FAIL: TestKeepsBinding
    record a2 bound to wrong file: got item-0000.out, want /tmp/x/item-0000.out
    record a1 bound to wrong file: got item-0001.out, want /tmp/x/item-0001.out
    ... (5 records)
FAIL

那两分钟的检查正是关键所在。一个你从未见过失败的回归测试只是一个假设,而不是一个保证。如果撤销修复后测试仍然通过(保持绿色),那么它测试的是别的东西,而那个别的东西通常是你本想保护的逻辑的副本。

捕获语义交换的断言

结构性检查从设计上就会漏掉这类错误,因此断言必须比较标识,而非形状。

在重排序发生前捕获预期的映射关系,然后在之后进行比较:

want := map[string]string{}
for _, item := range items {
    want[item.ID] = item.LocalPath      // snapshot before regrouping
}
// ... build manifest ...
for _, e := range entries {
    if e.Path != want[e.ID] {
        t.Errorf("%s bound to wrong file: got %s, want %s", e.ID, e.Path, want[e.ID])
    }
}

显式断言唯一性。 两个条目指向同一个构件是一种逐个条目检查无法发现的交换:

seen := map[string]string{}
for _, e := range entries {
    if prev, dup := seen[e.Path]; dup {
        t.Errorf("path %s shared by %s and %s", e.Path, prev, e.ID)
    }
    seen[e.Path] = e.ID
}

使用能实际重新排序的排序键。 一个各项内容已经排好序的测试固件什么也证明不了。将分组交错排列,这样排序操作就会跨越分组边界移动记录,而这正是触发该错误的条件。

检查扩展名,而不仅仅是路径。 在原始代码中,一个未转换的 LocalPath 仍然指向一个已删除的源文件。断言后缀可以捕获一整类“字段在转换后未更新”的错误。

这种模式还潜藏在何处

我是在音频管道中遇到这个问题的,但其模式是通用的。只要以下三者同时出现,你就要留意它:

  • “扁平化-处理-重新分组”的序列,当批量 API 比按组调用更经济时很常见。
  • 中间某个位置的原地排序或筛选,包括隐藏在非你编写的辅助函数中的操作。
  • 从循环计数器派生而非存储在记录中的产物名称。

它出现的具体场景有:批量处理图片然后按相册重新分组的缩略图生成;一次性渲染所有页面然后按章节组装的报告构建器;批量获取行、按时间戳排序以进行窗口化、然后按租户分区的 ETL 作业;任何通过 shell 调用并采用 --output-%d 模式的工具。

标志性的信号是类似“索引与排序后的顺序匹配”或“同一循环,同一索引”这样的注释。该注释记录了一个没有强制约束的不变量。它在编写时是成立的,但在下一次重构后会悄无声息地失效,而你的测试套件中没有任何东西会注意到这一点。

常见异议

“不要进行原地排序。” 这很合理,但你通常无法控制排序操作。在这里,它存在于一个共享的辅助函数中,被另一个依赖于该顺序的流水线使用。将其更改为返回一个副本会是一个影响范围更广的变更,有其自身的风险,而且也无法修复由位置派生名称这一根本问题。

“使用以 ID 为键的 map,而不是切片。” 这方法可行,并且完全消除了位置依赖。但它也带来了内存分配和顺序控制的成本,而当下一阶段以排序顺序进行流式处理时,顺序是很重要的。将路径存储在记录上可以在保留切片的同时获得同样的安全性。

“在事后验证输出。” 你可以这样做,但验证语义同一性通常意味着解码产物并比较内容,这既昂贵又常常无法在单元测试中实现。让绑定在结构上不可能被破坏,比检测破坏的成本更低。

“我们的类型是不可变的,所以我们无法存储路径。” 那么就返回一个将记录与产物配对的并行结构,并传递该结构,而不是传递两个需要你独立重新索引的切片。其原则不变:一个值承载两个部分。

重点总结

  1. 在“名称输入”和“名称输出”之间进行原地排序,会将从索引派生的文件名转变为一个静默的正确性 bug。
  2. 该 bug 会生成内容交换了的有效构件,因此存在性、大小、格式和模式检查都会通过。
  3. 添加所有权字段可以恢复分组,但无法恢复绑定。应将构件引用存储在记录上。
  4. 提取出纯函数,以便测试调用的是交付代码,然后将修复还原一次,以确认测试会失败。
  5. 断言同一性和唯一性,而不是形状,并使用其排序顺序会实际移动记录的固件 (fixtures)。

所有这些方法中最经济的是还原检查。如果你这个季度只写一个回归测试,那就写一个你亲眼见过它失败的测试。

常见问题解答

我如何知道我的流水线现在是否存在这个 bug? 找到所有使用循环索引来格式化文件名的地方。对于每一个地方,都要思考在该行代码与读回文件的代码行之间,切片(slice)是否可能被重新排序。如果中间存在排序、筛选、去重或并行分散(parallel scatter)操作,那么你就有了触发此 bug 的条件。然后编写上文提到的映射断言,看看它是否能通过。

静态分析器能捕获它吗? 不能。每一行代码本身都是正确的。这个 bug 存在于两个循环之间的关系中,而类型检查器没有理由将它们关联起来。这就是为什么缓解措施是结构性的,而不是一个 lint 规则。

这和差一错误(off-by-one)一样吗? 不一样,而且这个区别对于调试很重要。差一错误通常会在边界处崩溃,或产生一个明显错误的元素。而这个 bug 会产生一个完全的置换:每个元素都是错的,没有元素丢失,并且数量是正确的。如果你正在追踪一个“所有东西都有细微的错误,但没有任何东西损坏”的报告,那么置换(permutation)是比算术错误更好的假设。

这是否也适用于文件流水线之外的场景? 是的。任何时候你通过位置来维持两个集合之间的对应关系,重新排序都会破坏它。并行数组、以索引为键的缓存,以及批量 API 客户端中“第 N 个响应匹配第 N 个请求”的假设,都具有相同的模式。修复方法也相同:将两部分配对成一个值。