streamingfast / merger

Apache License 2.0
4 stars 5 forks source link

Forked blocks not deleted #33

Closed johnkozan closed 4 months ago

johnkozan commented 11 months ago

While running a firehose node, this warning appeared in my logs

Sep 22 21:29:21 firehose-eth-0 fireeth[212605]: 2023-09-22T21:29:21.546Z WARN (merger) cannot walk forked block files to delete old ones {"inclusive_low_boundary": 0, "inclusive_high_boundary": 18143900, "error": "processing object list: EOF"}

and forked block files in my object store were not being deleted.

The EOF error is generated by merger here: https://github.com/streamingfast/merger/blob/fe3ce68cb9f811f9694c0eeaf3dec203732a0991/merger_io.go#L278-L280

But should return dstore.StopIteration according to the comment here: https://github.com/streamingfast/dstore/blob/3924b3b36c778c14ef73ce108d965c774fb27fff/stores.go#L37-L39

I've made the change and tested it on my firehose node for the past week. The error message is gone and forked blocks are being deleted properly.

maoueh commented 11 months ago

I was surprised that just changing io.EOF to dstore.StopIteration solves the problem, I say this because while indeed dstore.StopIteration is better, the code still looks for this specific error after the walk:

https://github.com/streamingfast/merger/blob/fe3ce68cb9f811f9694c0eeaf3dec203732a0991/merger_io.go#L285-L291

After looking at your log, I just noticed that you see err as processing object list: EOF, it appears the s3 dstore implementation is wrong as it should have returned the error unmodified but it wrapped it, which is causing the err == io.EOF to never be true.

So, two things: