Fix re sync issue

[0xpetersatoshi]

Description of change

This fix removes the should_sync_stream function and associated logic which prevents the tap from re-syncing all the streams in the case where the previous run has exited with a failure. In the case where a state file is passed containing the currently_syncing key, the should_sync_stream would skip all the streams before reaching the one that matched currently_syncing and would only process that stream and any streams after it. Refactored to start syncing from the currently_syncing stream first and process all streams that also need to be synced. Additionally, modified logic that checks if a record should be written by comparing to the previous timestamp to only write the record if the timestamp is greater than the previous timestamp.

Manual QA steps

• Run the tap with a state file containing a currently_syncing stream. Ensure that all streams are still processed.

Risks

• N/A

Rollback steps

• revert this branch

[jacobrobertbaca]

I think we'll want to keep this as >= to account for multiple records being synced at the same timestamp.

e.g. if there are 10 records r at timestamp t=2 and on the first sync, we sync through only the first 4 of them, we want to ensure that those remaining 6 records aren't skipped over at the cost of us having to redundantly sync the first 4.

[0xpetersatoshi]

Good catch! My intent was to avoid re-syncing previously synced records on subsequent runs of the tap. But I missed that the write_bookmark function was updating the bookmark within the while loop that is batch processing records. What do you think about moving the write_bookmark function to the end of sync_endpoint after the batch processing has finished? I just moved it and tested it out it and it looks to work just fine that way as well. I updated the branch with the change. Let me know if that is acceptable.

[jacobrobertbaca]

Yep, I like that.

👍 sounds good to me.

[jacobrobertbaca]

included in #38