1191957 - Be smarter about splitting log files after periods of inactivity

Status: NEW

(Chat Core :: General, defect)

Description

[(inactive) Nihanth Subramanya [:nhnt11]]

Right now we split 30 minutes after the last message.
Some improvements that can be made:
- Split after the last non-system message (i.e. someone said something)
- Keep track of the rate of messages in the conversation:
  * Split if a new message arrives later than |(magic constant) * current rate|.
  * That way, if the current conversation is fairly active - say a message every
    5 minutes - we split after around 30 minutes of inactivity. If a conversation
    is less active - say 20 minutes between messages - we should split after an
    hour or two.
  * (Maybe) Don't split after just two messages - i.e. have a magic constant minimum number of messages required for a conversation to be split.

Comment 1

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Patch

I've used a magic multiplier of 4 in this patch. Also, the average time between messages will only be considered if there have been more than 2 messages. This should take care of the case when there are single messages every hour or more (i.e. not really part of a conversation). Should we try this in nightlies directly? I'm not sure if just me using it locally would give us much of a sample space to decide if it works well.

Comment 2

[Patrick Cloke [:clokep]]

Comment on attachment 8646664 [details] [diff] [review]
Patch

Review of attachment 8646664 [details] [diff] [review]:
-----------------------------------------------------------------

Maybe you weren't going to include it in this patch, but I think we also talked about not 'counting' system messages?

::: chat/components/src/logger.js
@@ +243,5 @@
>      let messageTime = aMessage.time * 1000;
>      let messageMidnight = new Date(messageTime).setHours(0, 0, 0, 0);
>  
> +    let timePerMessage =
> +      (this._lastMessageTime - this._startTime) / this._messageCount;

I suspect you actually want some sort of rolling average? Or will this restart each time we decide to 'close' a log file?

Comment 3

[aleth [:aleth]]

Comment on attachment 8646664 [details] [diff] [review]
Patch

Review of attachment 8646664 [details] [diff] [review]:
-----------------------------------------------------------------

+1 on not counting system messages.

I suspect this might break some tests, did you check?

::: chat/components/src/logger.js
@@ +232,5 @@
>    // We start a new log file in the following cases:
> +  // - If it has been more than 4x the current average time between messages,
> +  //   and there have been more than 2 messages to obtain the average time from.
> +  kInactivityMultiplier: 4,
> +  kMinimumMessageCount: 2,

I'd make this at least 4, for the average to be meaningful. Otherwise you likely run into edge cases. (You can consider this roughly as the minimum number of messages in an automatically split session.)

We should also wait a minimum time (30 minutes? an hour?) before we use any rate, to avoid sudden atypical bursts of activity distorting the result and leading to almost instant log splits.