~emersion/soju-dev

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
3 2

[PATCH] msgstore_fs: correctly handle timestamps

Details
Message ID
<20210304161430.44300-1-hubert@hirtz.pm>
DKIM signature
pass
Download raw message
Patch: +2 -4
---
 msgstore_fs.go | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/msgstore_fs.go b/msgstore_fs.go
index fdeb91c..a203ac3 100644
--- a/msgstore_fs.go
+++ b/msgstore_fs.go
@@ -340,8 +340,7 @@ func (ms *fsMessageStore) LoadBeforeTime(network *network, entity string, t time
		}
		copy(history[remaining-len(buf):], buf)
		remaining -= len(buf)
		year, month, day := t.Date()
		t = time.Date(year, month, day, 0, 0, 0, 0, t.Location()).Add(-1)
		t = truncateDay(t).AddDate(0, 0, -1)
	}

	return history[remaining:], nil
@@ -364,8 +363,7 @@ func (ms *fsMessageStore) LoadAfterTime(network *network, entity string, t time.
		}
		history = append(history, buf...)
		remaining -= len(buf)
		year, month, day := t.Date()
		t = time.Date(year, month, day+1, 0, 0, 0, 0, t.Location())
		t = truncateDay(t).AddDate(0, 0, 1)
	}
	return history, nil
}
-- 
2.30.1
Details
Message ID
<DJWaWapodDe9OS-IeID_StUjpGbKwexpc5xG3V7hKxqzncRCUndtLEHlqV9EQz6OEk-uX4s7R0R4SRK4pCA3z4GIc_S2k9S0TkMdkmID9pE=@emersion.fr>
In-Reply-To
<20210304161430.44300-1-hubert@hirtz.pm> (view parent)
DKIM signature
pass
Download raw message
What bug is this patch trying to fix? This computation is a bit finicky
IIRC.

CC delthas

> ---
>  msgstore_fs.go | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/msgstore_fs.go b/msgstore_fs.go
> index fdeb91c..a203ac3 100644
> --- a/msgstore_fs.go
> +++ b/msgstore_fs.go
> @@ -340,8 +340,7 @@ func (ms *fsMessageStore) LoadBeforeTime(network *network, entity string, t time
>  		}
>  		copy(history[remaining-len(buf):], buf)
>  		remaining -= len(buf)
> -		year, month, day := t.Date()
> -		t = time.Date(year, month, day, 0, 0, 0, 0, t.Location()).Add(-1)
> +		t = truncateDay(t).AddDate(0, 0, -1)
>  	}
>
>  	return history[remaining:], nil
> @@ -364,8 +363,7 @@ func (ms *fsMessageStore) LoadAfterTime(network *network, entity string, t time.
>  		}
>  		history = append(history, buf...)
>  		remaining -= len(buf)
> -		year, month, day := t.Date()
> -		t = time.Date(year, month, day+1, 0, 0, 0, 0, t.Location())
> +		t = truncateDay(t).AddDate(0, 0, 1)
>  	}
>  	return history, nil
>  }
> --
> 2.30.1
Details
Message ID
<---VZ8b0Ghgjr6BBlMKvxfBwnlodB42VjlhZB0cDCFyxtCWXABQFBfnKQBo64wjQDVTvsInwYyZ0hcqMoPtlJjMaolLt4sJt868wy8iNb3k=@emersion.fr>
In-Reply-To
<DJWaWapodDe9OS-IeID_StUjpGbKwexpc5xG3V7hKxqzncRCUndtLEHlqV9EQz6OEk-uX4s7R0R4SRK4pCA3z4GIc_S2k9S0TkMdkmID9pE=@emersion.fr> (view parent)
DKIM signature
pass
Download raw message
> What bug is this patch trying to fix? This computation is a bit finicky
> IIRC.
>
> CC delthas
>
> > ---
> >  msgstore_fs.go | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/msgstore_fs.go b/msgstore_fs.go
> > index fdeb91c..a203ac3 100644
> > --- a/msgstore_fs.go
> > +++ b/msgstore_fs.go
> > @@ -340,8 +340,7 @@ func (ms *fsMessageStore) LoadBeforeTime(network *network, entity string, t time
> >  		}
> >  		copy(history[remaining-len(buf):], buf)
> >  		remaining -= len(buf)
> > -		year, month, day := t.Date()
> > -		t = time.Date(year, month, day, 0, 0, 0, 0, t.Location()).Add(-1)
> > +		t = truncateDay(t).AddDate(0, 0, -1)

For future reference: this breaks in this case:

- The client requests history before 2021-03-08 12:08
- The bouncer loads all messages for that day
- It tries to load messages for the previous day, with AddDate this results in
  2021-03-07 12:08
- A message was sent at timestamp 2021-03-07 23:59, it's missed
Details
Message ID
<20210308122618.614b37cf@vroom.localdomain>
In-Reply-To
<---VZ8b0Ghgjr6BBlMKvxfBwnlodB42VjlhZB0cDCFyxtCWXABQFBfnKQBo64wjQDVTvsInwYyZ0hcqMoPtlJjMaolLt4sJt868wy8iNb3k=@emersion.fr> (view parent)
DKIM signature
pass
Download raw message
On Mon, 08 Mar 2021 11:10:40 +0000, Simon Ser wrote:
> > What bug is this patch trying to fix? This computation is a bit
> > finicky IIRC.
> >
> > CC delthas
> >  
> > > ---
> > >  msgstore_fs.go | 6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/msgstore_fs.go b/msgstore_fs.go
> > > index fdeb91c..a203ac3 100644
> > > --- a/msgstore_fs.go
> > > +++ b/msgstore_fs.go
> > > @@ -340,8 +340,7 @@ func (ms *fsMessageStore)
> > > LoadBeforeTime(network *network, entity string, t time }
> > >  		copy(history[remaining-len(buf):], buf)
> > >  		remaining -= len(buf)
> > > -		year, month, day := t.Date()
> > > -		t = time.Date(year, month, day, 0, 0, 0, 0,
> > > t.Location()).Add(-1)
> > > +		t = truncateDay(t).AddDate(0, 0, -1)  

I thought `Add(-1)` was meant to refer to the day before, but it really
means "the last instant of the day before", so
`today.Add(-1 * time.Nanosecond)`.

The filesystem message store has had some random issues where messages
are missed, but this line isn't the cause it seems.  This patch may be
discarded.
Reply to thread Export thread (mbox)