Skip to content

improvements to message replay code#477

Merged
slingamn merged 6 commits intoergochat:masterfrom
slingamn:issue457_firstpass.4
May 12, 2019
Merged

improvements to message replay code#477
slingamn merged 6 commits intoergochat:masterfrom
slingamn:issue457_firstpass.4

Conversation

@slingamn
Copy link
Copy Markdown
Member

@slingamn slingamn commented May 8, 2019

This checks the first five boxes on #457. It's a bit fiddly, lots of edge cases and inadequately tested code paths.

@slingamn
Copy link
Copy Markdown
Member Author

slingamn commented May 9, 2019

Basically, uh, I started implementing #475, then as part of that I found several bugs in the bouncer code, and the fixes conflicted with work in this branch so I just made them on top.

The most significant bug is that the connection limiter was "leaking" IPs for bouncer sessions, i.e., if you attached an additional session to a nick from an IP, then disconnected that session, its entry in the connection limiter would never be cleaned up.

I made some architectural changes to make bugs like this a little less likely. One of them is that on a reattach, the old goroutine exits as quickly as possible without trying to play the registration burst --- that's deferred to the new goroutine.

gencapdefs.py Outdated
identifier="EventPlayback",
name="draft/event-playback",
url="https://github.com/ircv3/ircv3-specifications/pull/362",
standard="Draft IRCv3",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be "proposed IRCv3" instead of draft, as it hasn't been accepted and merged in.

)

// a Tagmsg that consists entirely of junk tags is not stored
var junkTags = map[string]bool{
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may be good to rename this to transientTags, it's a bit more accurate and feels a lil less critical~

@DanielOaks
Copy link
Copy Markdown
Member

This looks like a veeeery fun one! Gonna be interesting to see how the bugs shake out of this as it's used in the real world ;D

After those few changes are made (Draft -> proposed, and renaming junkTags to transientTags or maybe unstorableTags for the extra clarity), totally happy for this to be merged in as-is. Nice on separating the history stuff out into a separate module as much as we're able to.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants