Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

joinEntry seems to traverse entire log in certain cases #1110

Closed
justin0mcateer opened this issue Oct 13, 2023 · 3 comments · Fixed by #1132
Closed

joinEntry seems to traverse entire log in certain cases #1110

justin0mcateer opened this issue Oct 13, 2023 · 3 comments · Fixed by #1132

Comments

@justin0mcateer
Copy link

In the event that we have been disconnected and there are concurrent updates, OpLog.joinEntry seems to cause OpLog.traverse to traverse the entire log to 'double check' that the entry isn't already in the log.

Perhaps I am mis-understanding, but the 'shouldStopFn' value seems that it will never match and thus the traverse will continue to the end of the log.

In the event that the new entry (Head) points directly to a log entry that already exists locally, this is stop very quickly and function as expected. This seems to correlate with the behavior that we see where updates (even thousands) are handled quite well, so long as all of the nodes are always connected when updates are happening. However, if a node is disconnected and there are updates, the node will hang traversing the entire previous log.

https://github.com/orbitdb/orbitdb/blob/main/src/oplog/log.js#L238

Why is this extra check applied? Is there a known condition where we might receive an entry which is in the log, but somehow not in the index, or is this just paranoia?

@justin0mcateer
Copy link
Author

Also, in 'joinEntry', why is all of this work done to see if the entry is in the log before checking if the entry is valid or that we are even allowed to add it to the log? It seems like those checks should come before the checking if it is already in the log. Perhaps this makes more sense if the log is extremely short?

@justin0mcateer
Copy link
Author

One last comment. OpLog.traverse calls 'fetch' in the process of traversing the log. In this case that I mentioned, won't this have the effect of causing it to replicate the entire log contents locally and isn't that not the intention?

@haydenyoung
Copy link
Member

Thanks for bringing these issues to our attention. We are actively reviewing them and hope to have a status update soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants