Julien ÉLIE <iulius@nom-de-mon-site.com.invalid> writes:

> Incidentally, is the "File Descriptor Limits" still accurate in INSTALL?

> INN won't be able to increase the limits above the hard limits set by
> your operating system; on some systems, that hard limit is normally
> 256 file descriptors (Linux, for example). On others, like Solaris,
> it's 1024. Increasing the limit beyond that value may require serious
> system configuration work. (On some operating systems, it requires
> patching and recompiling the kernel. On Solaris it can be changed in
> /etc/system, but for 2.6 or earlier the limit cannot be increased
> beyond 1024 without breaking select(2) and thereby breaking all of
> INN. For current versions of Linux, you may be able to change the
> maximum by writing to /proc/sys/fs/file-max.)

> Isn't the hard limit 1024 (or more?) for Linux, and 256 for Solaris?

1024 is now the default for Linux, it looks like, and you can now change
it dynamically using the normal ulimit command (no need for messing around
in proc). I have no idea what Solaris does these days. I suspect I last
tested that part of INN against Solaris 7, which is a very long time ago.

> I've found out that when systemd is in use, systemd/sd-daemon.h includes
> asm/socket.h when __USE_MISC is set, which in turn includes
> linux/posix_types.h which unconditionnaly redefines __FD_SETSIZE to 1024
> (even if it has another value) for use with __kernel_fd_set. So
> __FD_SETSIZE has to be forced before any inclusion of bits/types.h and
> after any inclusion of systemd/sd-daemon.h on Linux (systemd/sd-daemon.h
> also includes bits/types.h).

I'm a little worried that something is going to be unhappy with redefining
__FD_SETSIZE somewhere. That approach was always a bit of an intrusive
hack. But if Diablo is happy, maybe it will be fine.

> I am unsure of the consequences but systemd might not work with INN if a
> systemd file descriptor exceeds 1024 (?)

I suspect it will only matter if the notification file descriptor is over
1024, which shouldn't happen because it gets passed in on startup.

> Jesse, I'll also change the log line:
> xxx:yyy zzz free but was in SMASK
> to:
> xxx:yyy zzz free but was in SMASK; looks like fd_set is not large
> enough (you may want to increase FD_SETSIZE to a higher value by
> rebuilding INN with -DLARGE_FD_SETSIZE=4096)

We should be able to do better than this and detect the actual problem
before we corrupt our data structures, I think, by testing whether a file
descriptor is larger than FD_SETSIZE.

--
Russ Allbery (eagle@eyrie.org) <https://www.eyrie.org/~eagle/>

Please post questions rather than mailing me directly.
<https://www.eyrie.org/~eagle/faqs/questions.html> explains why.

Hi Russ,

>> Can we talk about the implications of setting icdsynccount to a much
>> much higher value than 10? I have it set to 500 now, but curious if
>> there could be gains by setting this really high in my scenario where I
>> know I'm not receiving duplicate articles and no articles exist on the
>> destination?
>
> It decreases the frequency of syncs so it's a straight tradeoff between
> faster speed and more data loss if innd crashes (or the system crashes, or
> loses power, etc.). The risk is that if something goes wrong and innd
> crashes before updating the history and active files, the sending system
> may think it has successfully transmitted articles (they were acked), but
> the receiving system never recorded they were received and thus while
> they're on disk they're orphaned.
>
> If innd doesn't crash, you could set it arbitrarily high and I think it
> would just make things faster. Think of it as the checkpointing
> frequency.

I'll improve the documentation of icdsynccount with some of your
explanations.
There's also an undocumented use of icdsynccount for buffindexed: the
head of the buffer is flushed every 10*icdsynccount+1 articles.
I can mention it in the documentation.

As far as I see, other overview and storage methods flush indexes or
headers for each article (tradindexed, cnfs, timecaf) or after a
configurable period of time (transtimelimit of 10s by default for
ovsqlite for instance).

For buffindexed, maybe a dedicated option would be better, than to reuse
icdsynccount? (buffindexedheadsync?)

> Make sure that you've disabled sync for all of your news logs. I think
> that's already covered in INSTALL but a lot of people forget about it.

I'll add a note too, following one of your next articles in this thread.

--
Julien ÉLIE

« Traversez la rivière en foule, le crocodile ne vous mangera pas. »
(proverbe malgache)

Hi Russ,

>> Isn't the hard limit 1024 (or more?) for Linux, and 256 for Solaris?
>
> 1024 is now the default for Linux, it looks like, and you can now change
> it dynamically using the normal ulimit command (no need for messing around
> in proc). I have no idea what Solaris does these days. I suspect I last
> tested that part of INN against Solaris 7, which is a very long time ago.

Solaris 10 and 11 have in sys/select.h:

#ifndef FD_SETSIZE
#ifdef _LP64
#define FD_SETSIZE 65536
#else
#define FD_SETSIZE 1024
#endif /* _LP64 */

so on Solaris 64-bit, the default is large enough :)

I bet we can now remove the documentation of the 256 limit for Solaris
as it no longer applies to current versions.

>> I am unsure of the consequences but systemd might not work with INN if a
>> systemd file descriptor exceeds 1024 (?)
>
> I suspect it will only matter if the notification file descriptor is over
> 1024, which shouldn't happen because it gets passed in on startup.

Yep you're right.

>> Jesse, I'll also change the log line:
>> xxx:yyy zzz free but was in SMASK
>> to:
>> xxx:yyy zzz free but was in SMASK; looks like fd_set is not large
>> enough (you may want to increase FD_SETSIZE to a higher value by
>> rebuilding INN with -DLARGE_FD_SETSIZE=4096)
>
> We should be able to do better than this and detect the actual problem
> before we corrupt our data structures, I think, by testing whether a file
> descriptor is larger than FD_SETSIZE.

Sure.
The check could be done in RCHANadd, SCHANadd, and WCHANadd.
It is when channels.max_fd and channels.max_sleep_fd are updated, when
needed.
We could then log a warn message each time these max_fd or max_sleep_fd
counts *would* have been updated and be higher than FD_SETSIZE-1? And
immediately close the channel?

This way, the select(channels.max_fd + 1, &rdfds, &wrfds, NULL, &tv)
call won't go beyond the expected limit.
But no new connection will be seen (they will probably timeout).

I see that innfeed already does something like that in endpoint.c:

#if defined(FD_SETSIZE)
if ((unsigned int) fd >= FD_SETSIZE) {
warn("ME fd (%d) looks too big (%d -- FD_SETSIZE)", fd,
FD_SETSIZE);
return NULL;
}
#else
if (fd > (sizeof(fd_set) * CHAR_BIT)) {
warn("ME fd (%d) looks too big (%d -- sizeof (fd_set) *
CHAR_BIT)",
fd, (sizeof(fd_set) * CHAR_BIT));
return NULL;
}
#endif

Would a similar check be added for the xCHANadd functions?
I guess the check could be done by a new function in lib/fdlimit.c that
can be used for both innd and innfeed, returning true/false.

--
Julien ÉLIE

« Traversez la rivière en foule, le crocodile ne vous mangera pas. »
(proverbe malgache)

On Jul 3, 2023 at 11:46:51 AM CDT, "Julien ÉLIE"
<iulius@nom-de-mon-site.com.invalid> wrote:

> Hi Russ,
>
>>> Isn't the hard limit 1024 (or more?) for Linux, and 256 for Solaris?
>>
>> 1024 is now the default for Linux, it looks like, and you can now change
>> it dynamically using the normal ulimit command (no need for messing around
>> in proc). I have no idea what Solaris does these days. I suspect I last
>> tested that part of INN against Solaris 7, which is a very long time ago.
>
> Solaris 10 and 11 have in sys/select.h:
>
> #ifndef FD_SETSIZE
> #ifdef _LP64
> #define FD_SETSIZE 65536
> #else
> #define FD_SETSIZE 1024
> #endif /* _LP64 */

Can confirm that Solaris 9 also has the same values.

Adding to my previous message:

>>> Isn't the hard limit 1024 (or more?) for Linux, and 256 for Solaris?
>>
>> 1024 is now the default for Linux, it looks like, and you can now change
>> it dynamically using the normal ulimit command (no need for messing
>> around
>> in proc).  I have no idea what Solaris does these days.  I suspect I last
>> tested that part of INN against Solaris 7, which is a very long time ago.
>
> Solaris 10 and 11 have in sys/select.h:
>
> #ifndef FD_SETSIZE
> #ifdef _LP64
> #define FD_SETSIZE      65536
> #else
> #define FD_SETSIZE      1024
> #endif  /* _LP64 */

I've came accross this page:
https://support.oracle.com/knowledge/Sun%20Microsystems/1005979_1.html

saying that the default fd limit is 256 up to Solaris 11.4 SRU 26.

There is a difference between the *stdio* and *select* interfaces, which
certainly explains the point in the recommendation of 256 for
rlimitnofile for Solaris.

stdio handles up to 256 file descriptors up to Solaris 10, and 32-bit
Solaris 11.3. For 64-bit Solaris 11.x, and 32-bit Solaris 11.4, stdio no
longer has that limit.

I then believe the recommandation for rlimitnofile should remain 256,
and can be increased only for Solaris 11.4 or 64-bit Solaris 11.0-11.3.
If of course I understand well what is said in the Oracle support page.

--
Julien ÉLIE

« Traversez la rivière en foule, le crocodile ne vous mangera pas. »
(proverbe malgache)

Hi all,

This thread is pretty interesting. A few improvements to do in our documentation,
besides a fix for the maximum number of file descriptors to use.

> I'll improve the documentation of icdsynccount with some of your
> explanations.

Here follows a suggestion, with a recommendation of running makehistory
for recovery. Any other caveat or recovery actions to recommend?

icdsynccount
How many article writes between updating the active and history
files. The default value is 10.

This is a trade-off between faster speed and more data loss if innd
crashes (or the system crashes, or loses power, etc.). The higher
this parameter is, the less frequent syncs are done. It somehow
represents a checkpointing frequency: it would be the maximum
number of articles stored on disk that might be orphaned in case of
a crash as they wouldn't have been recorded in the history file.
There would also be failures to store articles with already
assigned article numbers, but not recorded in the active file. (In
order not to encounter such errors, you should then rebuild the
history file and the overview with makehistory(8). The active file
will be automatically be renumbered after that operation.)

> There's also an undocumented use of icdsynccount for buffindexed: the
> head of the buffer is flushed every 10*icdsynccount+1 articles.
> I can mention it in the documentation.

I'll add a dedicated buffindexedheadsync parameter for that.

>> Make sure that you've disabled sync for all of your news logs.  I think
>> that's already covered in INSTALL but a lot of people forget about it.
>
> I'll add a note too, following one of your next articles in this thread.

Proposition for INSTALL and newslog(5):

If you're using syslogd, edit /etc/syslog.conf on your system and add
lines that look like the following:

news.crit <pathlog in inn.conf>/news.crit
news.err <pathlog in inn.conf>/news.err
news.notice -<pathlog in inn.conf>/news.notice

The minus sign as the first character for the path to news.notice
instructs syslogd not to synchronize the log file to disk every time
there is a new log entry (it otherwise degrades performance). Depending
on the syslog daemon you are using, log synchronization may already be
disabled by default (that is for instance the case for rsyslogd or
syslog-ng).

--
Julien ÉLIE

« – Obélix, mon garçon, quand tu viens chez moi, j'aimerais que tu
laisses ton menhir à la porte !
– Mais chef, le menhir se porte aussi bien à l'intérieur qu'à
l'extérieur !
– Sauf quand la porte est trop petite pour le faire entrer
imbécile !!! » (Astérix)

Julien ÉLIE <iulius@nom-de-mon-site.com.invalid> writes:

> I've came accross this page:
> https://support.oracle.com/knowledge/Sun%20Microsystems/1005979_1.html

> saying that the default fd limit is 256 up to Solaris 11.4 SRU 26.

> There is a difference between the *stdio* and *select* interfaces, which
> certainly explains the point in the recommendation of 256 for rlimitnofile
> for Solaris.

> stdio handles up to 256 file descriptors up to Solaris 10, and 32-bit
> Solaris 11.3. For 64-bit Solaris 11.x, and 32-bit Solaris 11.4, stdio no
> longer has that limit.

> I then believe the recommandation for rlimitnofile should remain 256, and
> can be increased only for Solaris 11.4 or 64-bit Solaris 11.0-11.3.
> If of course I understand well what is said in the Oracle support page.

INN does some terrifying things to try to handle cases where file
descriptors higher than 256 are possible but it still wants to use stdio.
I'm not sure how well that stuff works in practice, but it's been there
for forever and I think it's still enabled. See lib/reservefd.c.

In theory, this is supposed to allow increasing the file descriptor limit
above 256 on platforms such as Solaris that have this stdio limitation,
and it did work at least at one time. Note that, critically, it only is
implemented for innd; all the other programs have to either not use stdio
or have to not open more than 256 file descriptors. I think that might
apply to everything?

--
Russ Allbery (eagle@eyrie.org) <https://www.eyrie.org/~eagle/>

Please post questions rather than mailing me directly.
<https://www.eyrie.org/~eagle/faqs/questions.html> explains why.

Hi Russ,

>> I've came accross this page:
>> https://support.oracle.com/knowledge/Sun%20Microsystems/1005979_1.html
>
>> saying that the default fd limit is 256 up to Solaris 11.4 SRU 26.
>
>> There is a difference between the *stdio* and *select* interfaces, which
>> certainly explains the point in the recommendation of 256 for rlimitnofile
>> for Solaris.
>
>> stdio handles up to 256 file descriptors up to Solaris 10, and 32-bit
>> Solaris 11.3. For 64-bit Solaris 11.x, and 32-bit Solaris 11.4, stdio no
>> longer has that limit.
>
>> I then believe the recommandation for rlimitnofile should remain 256, and
>> can be increased only for Solaris 11.4 or 64-bit Solaris 11.0-11.3.
>> If of course I understand well what is said in the Oracle support page.
>
> INN does some terrifying things to try to handle cases where file
> descriptors higher than 256 are possible but it still wants to use stdio.
> I'm not sure how well that stuff works in practice, but it's been there
> for forever and I think it's still enabled. See lib/reservefd.c.

Oh indeed, I had not noticed this lib/reservefd.c file.
innd reserves 4 or 5 file descriptors at startup and this is
unconditionally enabled. It still works as otherwise a fatal error is
returned, and Fopen()/Fclose() are still in use for these file
descriptors for history (file, stats and dbz).

I'm wondering whether this is still useful nowadays (as it is only used
for 5 file descriptors in innd), and we couldn't just use
fopen()/fclose() for them...

> In theory, this is supposed to allow increasing the file descriptor limit
> above 256 on platforms such as Solaris that have this stdio limitation,
> and it did work at least at one time. Note that, critically, it only is
> implemented for innd; all the other programs have to either not use stdio
> or have to not open more than 256 file descriptors. I think that might
> apply to everything?

innfeed has its own parameter (stdio-max) to reserve the given number of
file descriptors for stdio. It is not used by default, but one can set
it so that network socket file descriptors are always higher than this
number.

--
Julien ÉLIE

« Sème du bonheur dans le champ du voisin, tu seras surpris de constater
ce que le vent fera produire au tien. » (Juliette Saint Gelais)

Julien ÉLIE <iulius@nom-de-mon-site.com.invalid> writes:

> I'm wondering whether this is still useful nowadays (as it is only used
> for 5 file descriptors in innd), and we couldn't just use
> fopen()/fclose() for them...

I believe the problem is that some of the things for which it uses stdio
(most notably history) might be closed and reopened, and if a new incoming
connection is handled between the close and reopen (I'm not sure if that's
possible), it could grab that low-numbered file descriptor and leave no
available file descriptors lower than 256, in which case stdio on those
platforms will fail.

The chosen number of file descriptors is intended to be enough for history
plus whatever else used stdio (I think probably the active file and
overview, although I don't think overview uses stdio any more).

--
Russ Allbery (eagle@eyrie.org) <https://www.eyrie.org/~eagle/>

Please post questions rather than mailing me directly.
<https://www.eyrie.org/~eagle/faqs/questions.html> explains why.

Julien ÉLIE <iulius@nom-de-mon-site.com.invalid> writes:

> Sure.
> The check could be done in RCHANadd, SCHANadd, and WCHANadd.
> It is when channels.max_fd and channels.max_sleep_fd are updated, when
> needed.
> We could then log a warn message each time these max_fd or max_sleep_fd
> counts *would* have been updated and be higher than FD_SETSIZE-1? And
> immediately close the channel?

That sounds right to me.

> I see that innfeed already does something like that in endpoint.c:

> #if defined(FD_SETSIZE)
> if ((unsigned int) fd >= FD_SETSIZE) {
> warn("ME fd (%d) looks too big (%d -- FD_SETSIZE)", fd,
> FD_SETSIZE);
> return NULL;
> }
> #else
> if (fd > (sizeof(fd_set) * CHAR_BIT)) {
> warn("ME fd (%d) looks too big (%d -- sizeof (fd_set) *
> CHAR_BIT)",
> fd, (sizeof(fd_set) * CHAR_BIT));
> return NULL;
> }
> #endif

> Would a similar check be added for the xCHANadd functions?

Yup, that's exactly what I was thinking of.

> I guess the check could be done by a new function in lib/fdlimit.c that
> can be used for both innd and innfeed, returning true/false.

Sounds great to me!

--
Russ Allbery (eagle@eyrie.org) <https://www.eyrie.org/~eagle/>

Please post questions rather than mailing me directly.
<https://www.eyrie.org/~eagle/faqs/questions.html> explains why.

Julien ÉLIE <iulius@nom-de-mon-site.com.invalid> writes:

> Here follows a suggestion, with a recommendation of running makehistory
> for recovery. Any other caveat or recovery actions to recommend?

> icdsynccount
> How many article writes between updating the active and history
> files. The default value is 10.

> This is a trade-off between faster speed and more data loss if innd
> crashes (or the system crashes, or loses power, etc.). The higher
> this parameter is, the less frequent syncs are done. It somehow
> represents a checkpointing frequency: it would be the maximum

"It is essentially the frequency of checkpoints: the maximum number of
articles that may be orphaned in case of a crash..."

> number of articles stored on disk that might be orphaned in case of
> a crash as they wouldn't have been recorded in the history file.
> There would also be failures to store articles with already
> assigned article numbers, but not recorded in the active
> file.

"The missing updates to the active file would cause other problems later,
such as duplicate article numbers and corresponding errors when storing
new articles."

> (In order not to encounter such errors, you should then
> rebuild the history file and the overview with
> makehistory(8). The active file will be automatically be
> renumbered after that operation.)

"(If innd has crashed, you can fix these errors by rebuilding the history
file and overview with makehistory(8)...."

> Proposition for INSTALL and newslog(5):

> If you're using syslogd, edit /etc/syslog.conf on your system and add
> lines that look like the following:

> news.crit <pathlog in inn.conf>/news.crit
> news.err <pathlog in inn.conf>/news.err
> news.notice -<pathlog in inn.conf>/news.notice

> The minus sign as the first character for the path to news.notice
> instructs syslogd not to synchronize the log file to disk every time
> there is a new log entry (it otherwise degrades performance). Depending
> on the syslog daemon you are using, log synchronization may already be
> disabled by default (that is for instance the case for rsyslogd or
> syslog-ng).

Yup, looks good.

--
Russ Allbery (eagle@eyrie.org) <https://www.eyrie.org/~eagle/>

Please post questions rather than mailing me directly.
<https://www.eyrie.org/~eagle/faqs/questions.html> explains why.

Hi Russ,
>> Here follows a suggestion, with a recommendation of running makehistory
>> for recovery. Any other caveat or recovery actions to recommend?
[...]
> "It is essentially the frequency of checkpoints: the maximum number of
> articles that may be orphaned in case of a crash..."

OK, thanks for the rewording wich indeed is far better :)
I'll update the documentation accordingly.

--
Julien ÉLIE

« Il vaut mieux un tapis persan volé qu'un tapis volant percé ! »
(Astérix)

On Jul 3, 2023 at 6:03:08 AM CDT, "Julien ÉLIE"
<iulius@nom-de-mon-site.com.invalid> wrote:

> I hope all of these changes will be useful, and you're happy with your
> new hardware setup Jesse!

Circling back to this after having spent time migrating to my shiny new
hardware (amazing how much faster things go when you jump 10 generations of
hardware!). I am building INN adding "-DLARGE_FD_SETSIZE=4096" to COPT in
Makefile.global (is that the correct way?).

I see a number of these warnings, but I'm unsure which value is actually being
used?

libtool: compile: cc -g -O2 -D_FORTIFY_SOURCE=2 -fstack-protector-strong
-DLARGE_FD_SETSIZE=4096 -I../include -c network.c -fPIC -DPIC -o
..libs/network.o
In file included from network.c:49:
.../include/portable/system.h:78:17: warning: 'FD_SETSIZE' macro redefined
[-Wmacro-redefined]
# define FD_SETSIZE LARGE_FD_SETSIZE
^
/usr/include/sys/select.h:61:9: note: previous definition is here
#define FD_SETSIZE 1024
^

As a side - I appreciate the discussion and clarification on some of the
tuning bits discussed. Hopefully someone else will find it useful as well.

On Jul 16, 2023 at 9:36:14 AM CDT, "Jesse Rehmer"
<jesse.rehmer@blueworldhosting.com> wrote:

> On Jul 3, 2023 at 6:03:08 AM CDT, "Julien ÉLIE"
> <iulius@nom-de-mon-site.com.invalid> wrote:
>
>> I hope all of these changes will be useful, and you're happy with your
>> new hardware setup Jesse!
>
> Circling back to this after having spent time migrating to my shiny new
> hardware (amazing how much faster things go when you jump 10 generations of
> hardware!). I am building INN adding "-DLARGE_FD_SETSIZE=4096" to COPT in
> Makefile.global (is that the correct way?).
>
> I see a number of these warnings, but I'm unsure which value is actually being
> used?
>
> libtool: compile: cc -g -O2 -D_FORTIFY_SOURCE=2 -fstack-protector-strong
> -DLARGE_FD_SETSIZE=4096 -I../include -c network.c -fPIC -DPIC -o
> .libs/network.o
> In file included from network.c:49:
> ../include/portable/system.h:78:17: warning: 'FD_SETSIZE' macro redefined
> [-Wmacro-redefined]
> # define FD_SETSIZE LARGE_FD_SETSIZE
> ^
> /usr/include/sys/select.h:61:9: note: previous definition is here
> #define FD_SETSIZE 1024
> ^
>
> As a side - I appreciate the discussion and clarification on some of the
> tuning bits discussed. Hopefully someone else will find it useful as well.

Hmm I'm seeing other strange behavior like "make tests" seems to stall out and
do nothing after getting to authprogs/ident. INN does start, but even actsync
is core dumping.

I see the following in kernel logs:

pid 32149 (actsync), jid 0, uid 8: exited on signal 6 (core dumped)
pid 37633 (ident.t), jid 0, uid 8: exited on signal 6 (core dumped)

When I take the "-DLARGE_FD_SETSIZE=4096" option out of Makefile.global
everything builds/tests/runs okay.

Hi Jesse,

> Hmm I'm seeing other strange behavior like "make tests" seems to stall out and
> do nothing after getting to authprogs/ident. INN does start, but even actsync
> is core dumping.
>
> When I take the "-DLARGE_FD_SETSIZE=4096" option out of Makefile.global
> everything builds/tests/runs okay.

Oh, that's bad. It seems that the large FD_SETSIZE patch does not work
as it should on your installation.
I do not see these problems with Debian, and I have also run "make
tests" with success on an OpenBSD system. No stalling nor core dump
seen. No macro-redefinition warning, and indeed that's a problem as the
whole point of the patch is to define FD_SETSIZE before
/usr/include/sys/select.h does it.

I consider that subject still open and not fixed then.

I unfortunately do not have access to a working FreeBSD system right now
to test. These servers are down on the Compile Farm
(https://cfarm.tetaneutral.net/machines/list/).

Yes, adding -DLARGE_FD_SETSIZE=4096 to COPT in Makefile.global is the
expected thing to do.

Could you please try to add in include/portable/socket.h the following
lines?

#include "config.h"
#include "portable/macros.h"

+#if LARGE_FD_SETSIZE > 1024
+# define FD_SETSIZE LARGE_FD_SETSIZE
+#endif
+ #include <errno.h>
#include <sys/types.h>

When done, do you still see macro-redefinition warnings? Does it work
better?

--
Julien ÉLIE

« Moi, je n'ai pas goûté le sel de cette plaisanterie. » (Astérix)

Adding to my previous message:

> Could you please try to add in include/portable/socket.h the following
> lines?
>
> +#if LARGE_FD_SETSIZE > 1024
> +#    define FD_SETSIZE LARGE_FD_SETSIZE
> +#endif
>
> When done, do you still see macro-redefinition warnings?  Does it work
> better?

You can then ignore macro-redefinition warnings between
include/portable/socket.h and include/portable/system.h; that would be fine.
If that works, I'll see how to handle that gracefully.

--
Julien ÉLIE

« Moi, je n'ai pas goûté le sel de cette plaisanterie. » (Astérix)

Hi Jesse,

> When I take the "-DLARGE_FD_SETSIZE=4096" option out of Makefile.global
> everything builds/tests/runs okay.

And normally you will no longer have errors like "free:-1 35 free but
was in SMASK", "SERVER cant store overview", "tradindexed: cannot get
offset for article", etc.
No channel with a file descriptor above 1024 will be created.

--
Julien ÉLIE

« Ne craignez pas d'être lent, craignez seulement d'être à l'arrêt. »

On Jul 29, 2023 at 7:11:02 AM CDT, "Julien ÉLIE"
<iulius@nom-de-mon-site.com.invalid> wrote:

> Hi Jesse,
>
>> Hmm I'm seeing other strange behavior like "make tests" seems to stall out and
>> do nothing after getting to authprogs/ident. INN does start, but even actsync
>> is core dumping.
>>
>> When I take the "-DLARGE_FD_SETSIZE=4096" option out of Makefile.global
>> everything builds/tests/runs okay.
>
> Oh, that's bad. It seems that the large FD_SETSIZE patch does not work
> as it should on your installation.
> I do not see these problems with Debian, and I have also run "make
> tests" with success on an OpenBSD system. No stalling nor core dump
> seen. No macro-redefinition warning, and indeed that's a problem as the
> whole point of the patch is to define FD_SETSIZE before
> /usr/include/sys/select.h does it.
>
>
> I consider that subject still open and not fixed then.
>
>
> I unfortunately do not have access to a working FreeBSD system right now
> to test. These servers are down on the Compile Farm
> (https://cfarm.tetaneutral.net/machines/list/).
>
>
> Yes, adding -DLARGE_FD_SETSIZE=4096 to COPT in Makefile.global is the
> expected thing to do.
>
>
>
>
> Could you please try to add in include/portable/socket.h the following
> lines?
>
> #include "config.h"
> #include "portable/macros.h"
>
> +#if LARGE_FD_SETSIZE > 1024
> +# define FD_SETSIZE LARGE_FD_SETSIZE
> +#endif
> +
> #include <errno.h>
> #include <sys/types.h>
>
>
>
> When done, do you still see macro-redefinition warnings? Does it work
> better?

Tested and I didn't see the warnings at compile time, 'make tests' is good,
and no more core dumping.

Hi Jesse,

>> When done, do you still see macro-redefinition warnings? Does it work
>> better? If that works, I'll see how to handle that gracefully.
>
> Tested and I didn't see the warnings at compile time, 'make tests' is good,
> and no more core dumping.

Thanks for the confirmation!

I've found out a graceful way to enforce header inclusions in the right
order, using our clang-format code reformatter to instruct it to begin
with config.h, then portable/system.h and then all the other headers in
alphabetical order.

The problem was portable/socket.h being otherwise included before
portable/system.h (because of the alphabetical order). This is now fixed!

https://github.com/InterNetNews/inn/commit/5f260cbb794c847c74739f0197344ef3f42a2367

I hope the -DLARGE_FD_SETSIZE build-time option now works fine, and you
haven't noticed any problem since last week.

--
Julien ÉLIE

« Se taire, c'est pareil dans toutes les langues. » (Philippe Geluck)

On Aug 4, 2023 at 3:38:25 PM CDT, "Julien ÉLIE"
<iulius@nom-de-mon-site.com.invalid> wrote:

> Hi Jesse,
>
>>> When done, do you still see macro-redefinition warnings? Does it work
>>> better? If that works, I'll see how to handle that gracefully.
>>
>> Tested and I didn't see the warnings at compile time, 'make tests' is good,
>> and no more core dumping.
>
> Thanks for the confirmation!
>
> I've found out a graceful way to enforce header inclusions in the right
> order, using our clang-format code reformatter to instruct it to begin
> with config.h, then portable/system.h and then all the other headers in
> alphabetical order.
>
> The problem was portable/socket.h being otherwise included before
> portable/system.h (because of the alphabetical order). This is now fixed!
>
>
> https://github.com/InterNetNews/inn/commit/5f260cbb794c847c74739f0197344ef3f42a2367
>
>
> I hope the -DLARGE_FD_SETSIZE build-time option now works fine, and you
> haven't noticed any problem since last week.

Everything has been humming along well. I'll give this new commit a test too.

I have a somewhat related question, why does the configure output for FreeBSD
say to use gmake? I think I've overlooked this the entire time I've used
FreeBSD and have always been using make.

Hi Jesse,

>> I hope the -DLARGE_FD_SETSIZE build-time option now works fine, and you
>> haven't noticed any problem since last week.
>
> Everything has been humming along well. I'll give this new commit a test too.

Thanks for the testing. (You need removing the additional #define
FD_SETSIZE added in portable/socket.h manually last week, as it is no
longer needed with this commit.)

> I have a somewhat related question, why does the configure output for FreeBSD
> say to use gmake? I think I've overlooked this the entire time I've used
> FreeBSD and have always been using make.

I've already came across FreeBSD versions on which stock make did not
work. I don't remember the versions but maybe current make versions
work fine.
According to INSTALL: "The INN Makefiles use the syntax include FILE,
rather than the syntax expected by some BSDish systems of .include
<FILE>. If your system expects the latter syntax, the recommended
solution is to install GNU make".

So in your case, if make works fine, you can just go on using it.

Instead of saying in the configure output:

Mandatory FreeBSD packages:
- gmake (INN needs to be built using gmake instead of make)
- autoconf (generate a configuration script)
- libtool (for library creation)
- perl5

It would probably be better to change the first requirement to:

- make (or gmake if a syntax error occurs during the build)

?

--
Julien ÉLIE

« Medicus curat, natura sanat. »

On Aug 5, 2023 at 1:10:12 AM CDT, "Julien ÉLIE"
<iulius@nom-de-mon-site.com.invalid> wrote:

> Hi Jesse,
>
>>> I hope the -DLARGE_FD_SETSIZE build-time option now works fine, and you
>>> haven't noticed any problem since last week.
>>
>> Everything has been humming along well. I'll give this new commit a test too.
>
> Thanks for the testing. (You need removing the additional #define
> FD_SETSIZE added in portable/socket.h manually last week, as it is no
> longer needed with this commit.)

Got it.

>> I have a somewhat related question, why does the configure output for FreeBSD
>> say to use gmake? I think I've overlooked this the entire time I've used
>> FreeBSD and have always been using make.
>
> I've already came across FreeBSD versions on which stock make did not
> work. I don't remember the versions but maybe current make versions
> work fine.
> According to INSTALL: "The INN Makefiles use the syntax include FILE,
> rather than the syntax expected by some BSDish systems of .include
> <FILE>. If your system expects the latter syntax, the recommended
> solution is to install GNU make".
>
>
> So in your case, if make works fine, you can just go on using it.
>
> Instead of saying in the configure output:
>
> Mandatory FreeBSD packages:
> - gmake (INN needs to be built using gmake instead of make)
> - autoconf (generate a configuration script)
> - libtool (for library creation)
> - perl5
>
> It would probably be better to change the first requirement to:
>
> - make (or gmake if a syntax error occurs during the build)
>
> ?

This makes sense... I don't know the history because I recently converted back
to FreeBSD after a very long time spent in in the Linux realm. I recall from
the past some of the non-GNU development tools behaved quite a bit
differently. I used to have a default process to install all the GNU tools
after OS install, but haven't done that on recent system builds. "make" is
part of the base FreeBSD system.