在异步的文件系统事件中同步加锁写入

PR 链接:https://github.com/apache/eventmesh/pull/4344

功能说明

当创建一个新的 WebHookConfig 时,会调用 writeToFile 方法,创建一个文件并写入数据。fileWatchRegister 可以监测文件的创建、修改和删除事件,并且在创建和修改的时候调用 cacheInit 方法,将 WebHookConfig 写入 cacheWebHookConfig

Bug 现象

正常情况下,复制 webhook.github.eventmesh.all 配置文件时,会产生 ENTRY_CREATE 和 ENTRY_MODIFY 两个事件,这符合预期:

Bug 出现在首次调用 insertWebHookConfig 端点创建 WebHookConfig 的时候,只能捕捉到 ENTRY_CREATE 事件(不正常,预期的行为应该也是两个事件):

此时文件还没有被写入数据,是一个空文件,因此 cacheInit 会向 cacheWebHookConfig 写入 null

然而,循环一次就结束了,fileWatchRegister 并没有在文件写入 WebHookConfig 数据后,再次调用 cacheInit 方法。因此,cacheWebHookConfig 中没有有效值,GitHub 的 Webhook 调用将无法检索到对应的 Webhook 配置。

初步解决方法

我在捕获到文件事件之后添加了一点延时,解决了这个 bug:

1
2
3
4
5
6
7
try {
assert service != null;
key = service.take();
Thread.sleep(1000);
} catch (InterruptedException e) {
log.error("Interrupted", e);
}

添加延时后,就可以捕获到 ENTRY_CREATE 和 ENTRY_MODIFY 两个事件了。for (final WatchEvent<?> event : key.pollEvents()) 循环将循环两次,且两次都能读取到完整的文件数据并创建正确的 WebHookConfig。这个行为是正确、符合预期的。

Bug 原因

当 JVM 获取文件系统事件的速度快于文件写入速度时,获取到的文件事件和数据就是不完整的;当 JVM 获取文件系统事件的速度慢于文件写入速度时,获取到的文件事件和数据才是完整的。

也就是说,当 fileWatchRegister 捕获到文件创建事件时,在没有添加延时的情况下,key.pollEvents() 只会返回一个 ENTRY_CREATE 事件,而没有等待足够的时间以使文件的内容完全写入磁盘。

理想情况下,文件系统事件是异步的,文件锁是同步的,加锁写入完成后,fileWatchRegister 应该继续捕获到 ENTRY_MODIFY 事件才对。没能继续监听,是因为 cacheInit 抛出的没有被捕获的异常向上传播,跳出了 while 循环,导致 WatchKey 没有被重置,service.take() 的调用被阻塞,无法获取新的事件。

然而,文件的写入速度是不可控的,添加固定的延时并不是一个稳妥的解决方法。我换了一台电脑,这个 bug 就没有完全复现。

更好的解决方法

我的代码应该更具有鲁棒性,一个更好的解决方案是使用适当的文件监视事件来确保文件已经完全创建并写入,然后再执行操作。所以我监听文件的 “ENTRY_MODIFY” 事件,而不是在 “ENTRY_CREATE” 事件中执行 cacheInit(file),这样可以更有把握地等待文件完全写入。

在不同步的操作次数中同步共享缓存

PR 链接:https://github.com/apache/eventmesh/pull/4344#issuecomment-1673594283

多线程的调试靠日志

反复调用 updateWebHookConfig 端点时出现的 NPE:

调试模式下无法复现此 NPE。简单打几个日志,fileWatchRegister 获取到的是空文件,每次端点请求触发两次文件修改事件,第一次 NPE,第二次正常:

依然是简单地在获取事件前添加延时来解决问题,两次端点请求,每次请求只触发一次文件修改事件:

文件加锁、获取文件、最终写缓存和文件修改事件次数的详细日志。文件写入完毕后,触发第二次文件修改事件,此次正常工作:

Bug 原因

标注处可以直观的观察到,调用 updateWebHookConfig 端点后,在 writeToFile 中创建 FileOutputStream 时,触发了一次文件修改事件,fileWatchRegister 在文件加锁并写入前就获取了空文件,并调用 cacheInitcacheWebHookConfig 写入了 null,导致 NPE:

解决方案

可以看到,在文件完成加锁前就已经抛出了 NPE,所以无法使用 tryLock 方法来判断文件状态。

此前,在插入配置时会产生 ENTRY_CREATE 和 ENTRY_MODIFY 事件,可以根据事件类型来区分对待;而更新配置时只产生 ENTRY_MODIFY 事件,与前者不同。因此,我使用 CountDownLatch 来通知文件写入已经完成,并在缓存初始化前等待文件写入完成的通知,即可解决问题。

不使用 Semaphore,而是 CountDownLatch,是因为 ENTRY_MODIFY 事件不止一次,而文件写入只有一次。只要第一次事件后能确保使用完整数据来更新缓存,后续的事件都是幂等的操作。

后续的事件发生时,CountDownLatch 将不会介入,此时由文件锁来加锁。文件锁在 try-with-resource 内部,CountDownLatch 在外部,是不冲突的。

为什么不用非空判断呢?确实挺简单的,但是治标不治本。在这个问题修复之后,我记录的另一个问题也被顺便修复了。复现方法是在调用 insertWebHookConfig 端点后,紧接着调用 deleteWebHookConfig 端点,会抛出 NoSuchFile 异常:

思考题:在高并发下保证任务的有序性

PR 链接:https://github.com/apache/eventmesh/pull/4344#issuecomment-1681615940

我们修复了这个端点本身的功能,那并发呢?虽然作为一个配置端口,正常是不用考虑这么高的并发量的,不会这么频繁的更新删除配置。但反过来说,作为一个配置端口,在保证可靠性的前提下,并不需要很高的并发性能。

为了保证 countDown 操作的原子性,要不再加把锁?

高并发调用 updateWebHookConfig 端点时,如果立即调用 deleteWebHookConfig 端点,此时 ENTRY_MODIFY 事件栈还没有全部跑完、cacheInit 方法还未执行完毕,也就是说 pending 的更新配置的任务还没有全部完成,就提前执行了删除配置的任务。如何保证任务执行的严格有序性?要不再加个任务队列?

想到这里,我是越想越兴奋,但你可能会觉得:这只是一个接口而已 —— 是的,我们没有必要保证它的顺序一致性,只需要保证数据的最终一致性即可。既然管理员在某一时刻删掉了一个配置,那么,在此时刻前下发且预计在此时刻后执行的配置更新任务都将没有意义。

从代码层面讲,为了将 file.isFile()cacheInit(file) 组合成一个原子操作,以避免 file 在通过 isFile () 判断后、阻塞在 cacheInit(file) 前时被删除,进而 cacheInit 时确保配置文件有效,只需要将 EVENT 的 dispatch 流程整体包裹在一个 synchronized 对象锁中即可,并使用此对象锁与 writeToFile 同步。

考虑到 cacheInit 方法中并没有使用传入的 file 来反序列化配置,而是再次使用了 BufferedReader 重新从磁盘读取文件,那么在 dispatch 流程初期 file 被实例化时是否包含了完整的文件数据并不重要,我们只需要使用它的 path。因此,实例化等更多层级就不需要被包裹在同步锁中,控制了锁的范围,保证了性能。

再仔细一想,既然 ENTRY_MODIFY 事件是在 writeToFile 中创建 FileOutputStream 时触发的,而后者已经被包裹在对象同步锁里了,也可以确保先完成写入文件再读取文件的顺序性,连 CountDownLatch 都可以省了。这也是同样的道理,尽管配置文件缓存的速度落后于配置文件磁盘 IO 的速度,但如果管理员在某一时刻想要更新一个配置,那么,在此时刻前下发且即将在此时刻后执行的缓存更新任务都是过期的。它们将会重复读取最新的配置文件,并执行幂等的写缓存操作。

大道至简。考虑到这个端点有限的并发量,目前的同步机制已经完全足够了。