Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

This fix appears to work in the tests I have run, but I am not confident that it actually always works.

it might be a good idea to consider different approaches to dealing with the fact that the wal lib does not allow truncating all of its elements. four that come to mind (the first two being very similar to each other):

1. every time there is a write to the WAL, add a "sentinel" element in addition to the metrics being added. the sentinel is a well-known value that is impossible to obtain from serializing a metric. when reading from the WAL, ignore the sentinels. when truncating all metrics from the WAL, make sure to leave the latest write's sentinel in the WAL.
2. when trunctaing all metrics from the WAL, add a sentinel first, and then truncate everything except the sentinel. (why shouldn't this be possible?)
3. change the wal library such as to allow truncating everything from the WAL.
4. don't use the WAL library. it seems to me that, at an abstract level, what we want for this buffer is an on-disk deque, i.e. the possibility to add to/read from/remove from the front/back.

!signed-cla

@nagaflokhu sorry for this being so slow! We do appreciate your effort on this! For moving forward with the PR, we need to check why the tests are failing now. If the tests were wrong initially, we need an explanation on why they were wrong and a fix to correct the test. Otherwise, if the tests are correct, we need to fix the code to get the tests to pass.
We also need to make sure that the present error is covered by a unit-test, i.e. add a test if there is none yet.

@srebhan I've now fixed the tests that were failing and added an assertion for regression purposes. I just ran the additional assertion on the code from the latest master branch commit, and it shows that the previous implementation was not working: accepting a transaction and ending it did not change the amount of entries in the buffer. the other assertion in the test I modified was passing because the mask had a length of 2.

@nagaflokhu I took the liberty to push some commits to your branch including a unit-test and some missing pieces for fixing the issue. What do you think?

Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip.
Downloads for additional architectures and packages are available below.

☺️ This pull request doesn't significantly change the Telegraf binary size (less than 1%)

@nagaflokhu I took the liberty to push some commits to your branch including a unit-test and some missing pieces for fixing the issue. What do you think?

very nice!

fwiw, I wonder if an alternative disk buffer design might not be simpler than having to manage a mask, but it looks to me like we're not going down that route, so let's move forward I guess.

An alternative WAL implementation will not happen anytime soon so let's fix this!

@mstrandboge and @skartikey approved this but as I'm involved in the code, please review both! Furthermore, review but do not merge yet until we get some confirmations it fixes the issue(s).

The proposed fix looks solid and appears to resolve the issue with the buffer not shrinking properly. It correctly maintains the mask, handles the empty buffer case more consistently, and adjusts offsets after truncation as expected.

While reading through the code, I considered checking whether the condition b.isEmpty = b.entries()-removeIdx <= 0 reliably detects when the buffer becomes empty. This would be a key area to investigate if shrinking were to fail. However, I later noticed that the TestDiskBufferTruncate test case already covers this scenario effectively. It verifies the buffer’s behavior across multiple batches and confirms that, when logically empty, no metrics are returned and the remaining masked entry is handled appropriately due to the WAL design.

The fix aligns well with what this test validates. Great work on this!

Thanks @skartikey for the thorough review!