Skip to content

Make MonitorUpdatingPersister change persist type based on size #3770

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

Open
TheBlueMatt opened this issue May 6, 2025 · 5 comments
Open

Make MonitorUpdatingPersister change persist type based on size #3770

TheBlueMatt opened this issue May 6, 2025 · 5 comments
Labels
good first issue Good for newcomers

Comments

@TheBlueMatt
Copy link
Collaborator

One thing that smaller LDK clients might want is to use MonitorUpdatingPersister in many cases, but not if the ChannelMonitor is only, say, a few KiB when serialized. This avoids the overhead of compaction later if the actual monitor being persisted is tiny anyway.

I think this should be quite easy to do so tagging as a good first issue.

@TheBlueMatt TheBlueMatt added the good first issue Good for newcomers label May 6, 2025
@0xZaddyy
Copy link

can i work on this sir?

@TheBlueMatt
Copy link
Collaborator Author

Sure!

@joostjager
Copy link
Contributor

This avoids the overhead of compaction later if the actual monitor being persisted is tiny anyway.

Is this significant if the channel monitor is small anyway?

@TheBlueMatt
Copy link
Collaborator Author

If the storage layer is remote and high latency I think it could be? I guess in theory if we're really careful about the lookup logic (and the KVStore is async) we could do the loading in parallel (though its still a few more requests) but we'd still have to wait on compaction (without a background-compaction task running, which would require a generic spawn method...). Seems much easier to just check the length and skip some code for the common case :)

@joostjager
Copy link
Contributor

If I understand it correctly, this is special casing for channels at the beginning of their life and when they grow, the node will switch over to persisting updates and compaction anyway?

To me it isn't quite clear if this is really worth it. If the hit is significant, it is also a problem when the channel has outgrown the 'small' stage anyway?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants