Re: [PATCH] BUG/MINOR: log: GMT offset not updated when entering/leaving DST
Hello Willy,
Le 12/03/2016 07:54, Willy Tarreau a écrit :
>> +/* Return the GMT offset for a specific local time.
>> + * The string returned has the same format as returned by strftime(...
>> "%z", tm).
>> + * Offsets are kept in an internal cache for better performances.
>> + */
>> +const char *get_gmt_offset(struct tm *tm)
>> +{
>> +/* Cache offsets from GMT (depending on whether DST is active or not) */
>> +static char gmt_offsets[2][5+1] = { "", "" };
>> +
>> +int old_isdst = tm->tm_isdst;
>> +char *gmt_offset;
>> +
>> +/* Pretend DST not active if its status is unknown, or strftime() will
>> return an empty string for "%z" */
>> +if (tm->tm_isdst < 0) {
>> +tm->tm_isdst = 0;
>> +}
>> +
>> +/* Fetch the offset and initialize it if needed */
>> +gmt_offset = gmt_offsets[tm->tm_isdst & 0x01];
>> +if (unlikely(!*gmt_offset)) {
>> +strftime(gmt_offset, 5+1, "%z", tm);
>> +}
>> +
>> +/* Restore previous DST flag */
>> +tm->tm_isdst = old_isdst;
>> +
>> +return gmt_offset;
>> +}
> Since the cache is local to the function and initialized on the fly, no
> call is performed anymore on startup, it will fail on some chrooted
> systems because when the first log is emitted, you don't have access
> to the timezones anymore.
You just need to call tzset() once before chrooting to initialize the timezone
subsystem.
In fact, it's probably done by chance, there are a handful of functions that
call tzset implicitly.
I tested on a chroot environment on Linux/x86 and it works, but I admit it's
fragile.
I could cook an amended patch with an explicit call to tzset at init time.
> Thus I'd suggest to move the static gmt_offset out of the function
> and to have init_gmt_offset() which sets the string for both values
> of tm_isdst and which is called upon startup, and get_gmt_offset()
> which simply returns the correct one. At this point it will be so
> small that it's worth inlining it.
I obviously thought about a solution like that, but it's not that simple:
tm_isdst it not an isolated flag.
It cannot be interpreted without looking at a full struct tm.
tm_isdt can only be 0 only during the winter, and can be 1 only during the
summer.
For countries without a DST, the only valid value is 0.
Combining random dates with random values of tm_isdst has undefined behavior
and could lead to some portability problems.
In fact, there exists only one hour during which it can be 0 or 1: when going
from summer time to winter time for timezones that have a DST.
Think about it: there will exist two "Oct 30 2016, 02:30" in France, because
time will go backward once at 03:00.
This is the only time when both values of tm_isdst are valid at the same time,
and it's that value that allows to distinguish between those two equivalent
times.
That's why I chose to have a cache, so that I can use "real" times to compute
GMT offsets.
> An extra benefit of proceeding like this is that when we move to
> multi-threading, we won't have to lock for something which will only
> have to be initialized twice :-)
Wow, I didn't think haproxy would move to multi-threading one day.
I guess you'll have some local thread contexts (TLS or explicit context) where
the cache would fit nicely.
The cache would be initialized only once per thread and I guess you won't need
more threads than the number of CPU cores.
> Thanks!
> Willy
Let me know if you want a patch with an explicit call to tzset().
Benoît GARNIER
Re: Only using map file when an entry exists
Thanks Nanad,
That works perfectly, thank you
On 11 March 2016 at 22:37, Nenad Merdanovic wrote:
> Hello Neil,
>
> You seem to have missed my answer, so I am gonna top post this time :)
>
> http-request redirect location
> %[hdr(host),map(/etc/haproxy/redirect_host.map)] code 301 if {
> hdr(host),map(/etc/haproxy/redirect_host.map) -m found }
>
> Regards,
> Nenad
>
> On 03/11/2016 11:32 PM, Neil - HAProxy List wrote:
> > Hello
> >
> > I've left a little time and no one has said anything more so time for me
> > to act and submit a patch.
> >
> > I want to make functions that can be used in acls and take a map and
> > provide has_key and, for completeness, has_value
> >
> > Are those names uncontroversial/ suitable and, i really hope, is this
> > unnecessary as it already exists.
> >
> > I'm more that a little surprised to find myself the first to want this
> >
> > Cheers
> >
> > Neil
> >
> > On 11 Mar 2016 22:16, "Neil" > > wrote:
> >
> > Hello
> >
> > I've left a little time and no one has said anything more so time
> > for me to act and submit a patch.
> >
> > I want to make functions that can be used in acls and take a map and
> > provide has_key and, for completeness, has_value
> >
> > Are those names uncontroversia/ suitablel and, i really hope, is
> > this unnecessary as it already exists.
> >
> > I'm more that a little sutprised to find myself the first to want
> this
> >
> > Cheers
> >
> > Neil
> >
> > On 3 Mar 2016 18:08, "Neil - HAProxy List"
> > > > wrote:
> >
> > Thanks Conrad,
> >
> > That sort of thing looks better that what I had, and I'll give
> > it a go.
> >
> > I still think this is a bit long winded syntax for something
> > that probably quite a common things to want to do? A
> > map_contains type boolean function still seems like a good to
> have?
> >
> > Thanks
> >
> > Neil
> >
> > On 3 March 2016 at 13:05, Conrad Hoffmann > > wrote:
> >
> > If you are using haproxy >=1.6, you might be able to do
> > something like this:
> >
> > acl no_redir %[req.redir] -m str NO_REDIR
> > http-request set-var(req.redir) \
> > %[hdr(host),map(/etc/haproxy/redirect_host.map,NO_REDIR)]
> > http-request redirect location %[req.redir] code 301 if
> > !no_redir
> >
> > This is completely made up and untested, but I hope you get
> > the idea.
> > Avoids a second map lookup altogether, but also map lookups
> > are quite fast,
> > so unless you map is huge you don't really need to worry
> > about this. Also,
> > double negation, but this is just to give you some idea
> >
> > Cheers,
> > Conrad
> > --
> > Conrad Hoffmann
> > Traffic Engineer
> >
> > SoundCloud Ltd. | Rheinsberger Str. 76/77, 10115 Berlin,
> Germany
> >
> > Managing Director: Alexander Ljung | Incorporated in England
> > & Wales
> > with Company No. 6343600 | Local Branch Office | AG
> > Charlottenburg |
> > HRB 110657B
> >
> >
>
Re: [PATCH] BUG/MINOR: log: GMT offset not updated when entering/leaving DST
Hi Benoit, On Sat, Mar 12, 2016 at 09:45:25AM +0100, Beno?t GARNIER wrote: > > Since the cache is local to the function and initialized on the fly, no > > call is performed anymore on startup, it will fail on some chrooted > > systems because when the first log is emitted, you don't have access > > to the timezones anymore. > You just need to call tzset() once before chrooting to initialize the > timezone subsystem. > In fact, it's probably done by chance, there are a handful of functions that > call tzset implicitly. > I tested on a chroot environment on Linux/x86 and it works, but I admit it's > fragile. > I could cook an amended patch with an explicit call to tzset at init time. Yes I'd appreciate it then. > > Thus I'd suggest to move the static gmt_offset out of the function > > and to have init_gmt_offset() which sets the string for both values > > of tm_isdst and which is called upon startup, and get_gmt_offset() > > which simply returns the correct one. At this point it will be so > > small that it's worth inlining it. > I obviously thought about a solution like that, but it's not that simple: > tm_isdst it not an isolated flag. > It cannot be interpreted without looking at a full struct tm. > tm_isdt can only be 0 only during the winter, and can be 1 only during the > summer. > For countries without a DST, the only valid value is 0. > Combining random dates with random values of tm_isdst has undefined behavior > and could lead to some portability problems. OK I wasn't aware of this, that makes sense then. > In fact, there exists only one hour during which it can be 0 or 1: when going > from summer time to winter time for timezones that have a DST. > Think about it: there will exist two "Oct 30 2016, 02:30" in France, because > time will go backward once at 03:00. Yep definitely. The usual 90k seconds day :-) > This is the only time when both values of tm_isdst are valid at the same > time, and it's that value that allows to distinguish between those two > equivalent times. > That's why I chose to have a cache, so that I can use "real" times to compute > GMT offsets. That makes sense, thanks for explaining. > > An extra benefit of proceeding like this is that when we move to > > multi-threading, we won't have to lock for something which will only > > have to be initialized twice :-) > Wow, I didn't think haproxy would move to multi-threading one day. Don't be too excited, it will take decades :-) > I guess you'll have some local thread contexts (TLS or explicit context) > where the cache would fit nicely. Possibly, yes. At least it won't be static in the function anymore. > The cache would be initialized only once per thread and I guess you won't > need more threads than the number of CPU cores. On this last point we don't know. Once we get some shared memory between all processes, it could make sense to have many threads to benefit from some slow external operations (HW crypto accelerators for example, which may require tens to hundreds of threads to be useful). > > Thanks! > > Willy > Let me know if you want a patch with an explicit call to tzset(). Yes, please do it and I'll merge it with the upcoming release. Thanks! Willy
Re: There is kind of a spam issue on this ML no?
Oh don't say that ! Last time I did, I was told that I had a bad spirit, and that my antispam was just bad. The thing is : unlike other popular mailing-lists, ANYTHING or ANYONE can post on this one without prior subscription, hence that huge spammy noise. Hoggins! Le 11/03/2016 10:00, Arnaud B. a écrit : > On the last 2 or 3 days : > https://lut.im/jsIGNMzLDL/OuRdpkM9ZpVTIH47 signature.asc Description: OpenPGP digital signature
Re: [PATCH] BUG/MINOR: log: GMT offset not updated when entering/leaving DST
Am 12-03-2016 10:50, schrieb Willy Tarreau: Hi Benoit, On Sat, Mar 12, 2016 at 09:45:25AM +0100, Beno?t GARNIER wrote: > On Sat, Mar 12, Willy Tarreau wrote: [snipp] > An extra benefit of proceeding like this is that when we move to > multi-threading, we won't have to lock for something which will only > have to be initialized twice :-) Wow, I didn't think haproxy would move to multi-threading one day. Don't be too excited, it will take decades :-) Well when you stay tuned you well see a multi-threaded haproxy also ;-) I think this will be a major version change worth ;-). cheers Aleks
Re: There is kind of a spam issue on this ML no?
On Sat, Mar 12, 2016 at 11:08:08AM +0100, Hoggins! wrote: > Oh don't say that ! > Last time I did, I was told that I had a bad spirit, and that my > antispam was just bad. > > The thing is : unlike other popular mailing-lists, ANYTHING or ANYONE > can post on this one without prior subscription, hence that huge spammy > noise. Yes and that's the point. If you have ever been rejected from a development mailing list because you were not subscribed, you know how painful it is. And precisely the fact that the list is open means you *don't* have to be subscribed to post messages. So in short, people who complain about spam are those who are willing to receive these spam and I invite them to unsubscribe. They cause more noise on this list than the spams because you have to read their e-mails before knowing you're wasting your time. Willy
