I agree that the binary compatibility checks are crucial. But I kind of find it strange if one gets an assert failure some where if one tries to change MAX_CLIENTS. It is then not straight 
forward to relate both things and understand the consequences. That's why I put a comment next to the definition of MAX_CLIENTS saying:

/* note that if you change any of the following items, the ODB and the event shared memory buffers 
   become binary incopatible and one has to recompile ALL programs which are locally connected to the 
   ODB and to event buffers */

I think this is more descriptive than just a failing assert. 

If you look carefully in my proposal below, you will see that I rather used

sizeof(INT)

and not 

sizeof(int)

since as KO stated correctly sizeof(int) can change between different architectures. The derived type INT (all uppercase) has been carefully designed to have 32 bits on all architectures. So 
it will NOT change between them. If it does change, then we have a principal problem and many more things will break down. We should therefore have something like

if (sizeof(INT) != 4) then severe_error_and_stop_all_programs()

Now given that sizeof(INT) is everywhere the same, we can use it in the test

sizeof(BUFFER_HEADER) == NAME_LENGTH + 7*sizeof(INT) + MAX_CLIENTS *(NAME_LENGTH+13*sizeof(INT)+sizeof(float)+2*sizeof(DWORD)+MAX_EVENT_REQUESTS*4*sizeof(INT))

which then basically tests the structure byte alignment and padding. The comment above should warn users to change MAX_CLIENTS without thinking. 

Another strategy would be to put sizeof(BUFFER_HEADER) as the first two byes of the structure itself. We can the dynamically test the size of each bm_open_buffer(), and if the local size 
differs from the one saved in the buffer header, the program refuses to start, so we know exactly which program should have to be recompiled. The downside of this would be that the 
header structure has to be changed and we break binary compatibility with all existing programs. But maybe we should do this step once and be safe in the future.

Stefan


> The checks for byte sizes of critical data structures have been added to ensure (enforce) binary compatibility
> of midas with itself on different platforms (32-bit and 64-bit intel, on PPC, on ARM, etc).
> 
> This has worked well in the past and helped avoid problems and subtle bugs in the transition
> from 32-bit to 64-bit machines a few years ago. Of course now 32-bit machines are back
> as ARM CPUs and FPGA synthetic CPUs.
> 
> Replacing the checks with "computed" values will defeat this purpose because the values may be computed
> differently on different machines.
> 
> Specifically as proposed by Stefan, sizeof(int) can change depending on the target machine and depending
> on the compiler settings.
> 
> Of course this needs to be balanced against flexibility to adjust important settings like MAX_CLIENTS and MAX_EVENT_REQUESTS.
> 
> I would say the present system is just fine. You can change MAX_CLIENTS, rebuild MIDAS and it will not run (assert failure) giving
> you an indication that you are doing something non-trivial that will cause problems if you do it without thinking about it.
> 
> For example, one may think nothing of changing midas.h and recompiling MIDAS. But having to change odb.c
> may ring the little bell to tell you that you *also* have to rebuild *all* of your frontends. Even one unrebuilt frontend
> will corrupt all shared memory and crash everything.
> 
> I guess one other way to look at this is as a balance between something a few people do rarely against
> a function that protects everybody all the time.
> 
> That said, I think the checks should be reworked, instead of an assert failure they should give the error message
> and tell the user exactly what number to adjust in the size test. Also some checks are obsolete, there is no longer
> need to check the size of many ODB structures (equipment, etc). Once we are done with the db_get_record() rework,
> only checks for data structures in shared memory shall remain.
> 
> As the bottom line, to change MAX_CLIENTS, you already have to edit midas.h, asking you to also edit odb.c does
> not add much to the burden.
> 
> P.S. We are thinking how to make all these values dynamically changable, but basically it requires rolling out
> a new binary-incompatible version of MIDAS with added bugs. Maybe some day.
> 
> K.O.
> 
> 
> > The sizeof checks were originally invented by KO to check for binary compatibility between processes attached to the same ODB and event buffers. So if a 
> > compiler generates different structure sizes due to different padding, one would see that immediately. I wonder however if the absolute numbers make sense 
> > here. We could replace the 16444 by
> > 
> > NAME_LENGTH + 7*sizeof(INT) + MAX_CLIENTS *(NAME_LENGTH+13*sizeof(INT)+sizeof(float)+2*sizeof(DWORD)+MAX_EVENT_REQUESTS*4*sizeof(INT))
> > 
> > which makes this value automatically scale when one changes MAX_CLIENTS.
> > 
> > People of course have to be aware that if one changes MAX_CLIENTS, then all programs connected to the same ODB or event buffer need to be re-compiled 
> > and the ODB needs to be re-created from an ASCII file, but at least this would avoid tedious manual calculations.
> > 
> > Any opinion?
> > 
> > Stefan
> > 
> > 
> > > Below are the steps we used to increase the maximum number of frontends that we could run.
> > > 
> > > In midas.h
> > > 
> > > #define MAX_CLIENTS            64
> > > 
> > > changed to
> > > 
> > > #define MAX_CLIENTS            128
> > > 
> > > In msystem.h:
> > > 
> > > #define MAX_RPC_CONNECTION     64
> > > 
> > > changed to
> > > 
> > > #define MAX_RPC_CONNECTION     128
> > > 
> > > In odb.c:
> > > 
> > > assert(sizeof(BUFFER_HEADER) == 16444); 
> > > 
> > > GUESS: 256*64+60 = 16444, so change 64 to 128
> > > 
> > > changed to:                                                                                                                         
> > > 
> > > assert(sizeof(BUFFER_HEADER) == 32828); //256*128+60
> > > 
> > >  
> > > 
> > > DATABASE_HEADER = 64 + 64*DATABASE_CLIENT = 64 + 64*8256 = 528448
> > > 
> > > changed to:
> > > 
> > > DATABASE_HEADER = 64 + 128*DATABASE_CLIENT = 64 + 128*8256 = 1056832.

> if (sizeof(INT) != 4) then severe_error_and_stop_all_programs()

Quick reply.

Today, for fixed size data types one should use uint32_t & co, see
stdint.h
https://en.wikipedia.org/wiki/C_data_types#stdint.h
https://en.wikipedia.org/wiki/C99 (scroll down and click to open "implementation -> compiler support"

The other popular convention is "u32" used by the Linux kernel, you will see it in the linux kernel drivers.

If I remember right, WORD and DWORD grow legs from the 16-bit Motorolla 68xxx processors,
VxWorks and the VME bus. At some point the data buses were 16-bit wide and that we the WORD.

(I do not think UNIX ever used the WORD/DWORD names, i.e. MacOS has int32_t and u_int32_t).

K.O.

The type INT has been defined in 1989 when I for the first time sent data between a 16-bit MS-DOS computer and a 32-bit VAX computer (good old 
days!). At that time, uint32_t was not available at all. So much for the historical background.

I agree that switching from INT to int32_t is getting closer to standards and might help new people better understand things. This means however to 
touch all midas files and change about 5000 (!) locations:

BYTE -> uint8_t
WORD -> uint16_t
DWORD -> uint32_t
INT -> int32_t

Next we have the midas data types TID_xxx?

The nice thing now is that for example WORD and TID_WORD belong together and this is obvious. For uint16_t and TID_WORD is is not so obvious 
any more, so I guess we should rename TID_WORD to TID_UINT16_t. The same fore 

TID_BYTE -> TID_UINT8_T
TID_SBYTE -> TID_INT8_T
TID_WORD -> TID_UINT16_T
TID_DWORD -> TID_UINT32_T
TID_INT -> TID_INT32_T

But if we changer TID_XXX, the ASCII representations of the ODB break compatibility! Right now we have for example

[/Experiment]
midas http port = INT : 8080

which will become

[/Experiment]
midas http port = INT32_T : 8080

so one cannot load old ODB files any more!

With JSON encoding it's better because only the type number is stored, not the string. So INT -> 7 could stay, although in my opinion encoding the 
type in an integer number is not good for readability. Nobody knows what "7" means as a type. You always have to do a look-up in midas.c and count 
array indices manually.

I'm not sure how many experiments use the ASCII ODB format in one way or the other in some custom scripts. It might be that changing the format 
might have severe side effects for some experiments, so before we undertake this endeavor I would like to get some feedback here on the forum 
about people from other experiments and see what they think.

Stefan

> > if (sizeof(INT) != 4) then severe_error_and_stop_all_programs()
> 
> Quick reply.
> 
> Today, for fixed size data types one should use uint32_t & co, see
> stdint.h
> https://en.wikipedia.org/wiki/C_data_types#stdint.h
> https://en.wikipedia.org/wiki/C99 (scroll down and click to open "implementation -> compiler support"
> 
> The other popular convention is "u32" used by the Linux kernel, you will see it in the linux kernel drivers.
> 
> If I remember right, WORD and DWORD grow legs from the 16-bit Motorolla 68xxx processors,
> VxWorks and the VME bus. At some point the data buses were 16-bit wide and that we the WORD.
> 
> (I do not think UNIX ever used the WORD/DWORD names, i.e. MacOS has int32_t and u_int32_t).
> 
> K.O.

We had the problem in our lab that a frontend took about 6 seconds to gracefully 
shut down, mainly it needed to park some motors. I found that the shutdown command 
had a hard-coded timeout of 5 seconds, after which the frontend gets killed, and 
cannot finish the park operation. I change the code so that the client timeout 
stored in the ODB is taken instead of the hard-coded 5 seconds. This allows each 
client to fine-tune its timeout, to allow graceful shutdown, but also not let the 
user wait too long if the client gets stuck and needs a hard kill.

The default timeout for mfe.cxx based frontends has been changed to 10 seconds 
now, but in the frontend_init function this can be changed by the user code 
easily.

I hope this char does not trigger any bad side effects, but if it does, please 
report here.

Stefan

Due to some request several limitations like the maximal number of clients to the ODB have 
been increased in midas.h and committed to CVS. It is important to note that clients compiled
with the old limits cannot coexist with clients compiled with the new limits. You will get
ODB corruption notifications and everything will crash, and you wonder where this comes from.

So once you CVS update midas.h, revision 1.139, please make sure to recompile *ALL* your
midas applications with the new midas.h.

Stefan

> It is important to note that clients compiled
> with the old limits cannot coexist with clients compiled with the new limits. You will get
> ODB corruption notifications and everything will crash, and you wonder where this comes from.
> 
> So once you CVS update midas.h, revision 1.139, please make sure to recompile *ALL* your
> midas applications with the new midas.h.

Stefan, to avoid confusion from crashes caused by incompatible ODBs would it be possible to add a "version number" to ODB, together with a check and an error message 
saying "oops... incompatible ODB, please rebuild your programs"? We tend to have different versions of midas floating around and users have old executables stashed away, 
and all this makes it rather difficult to manually keep track on what ODB is compatible with what midas.

K.O.

> Stefan, to avoid confusion from crashes caused by incompatible ODBs would it be possible to add a "version number" to ODB,
together with a check and an error message 
> saying "oops... incompatible ODB, please rebuild your programs"? We tend to have different versions of midas floating around and
users have old executables stashed away, 
> and all this makes it rather difficult to manually keep track on what ODB is compatible with what midas.

I fully agree that a version number in ODB is a good thing, and I certainly will put one there, but this won't help for old
applications. If I add new code which checks in cm_connect_experiment() if the version number matches, this will only help for new
applications connecting to old ODBs. If old applications (prior to invention of the version number) connect to a new ODB, they still
will crash.

However, we are planning to make a new release 1.9.5 soon (next week), so can can people tell not to "mix" 1.9.5 with pre-1.9.5
programs.

> However, we are planning to make a new release 1.9.5 soon (next week), so can can people tell not to "mix" 1.9.5 with pre-1.9.5 programs.

Right. We cannot fix the past, but we should fix the future. BTW, "do not mix versions" is hard to enforce and mismatches did, do and
will happen. For one thing, looking at a given midas-using executable, how do I tell what version of midas it has inside?

K.O.

> Right. We cannot fix the past, but we should fix the future. BTW, "do not mix versions" is hard to enforce and mismatches did, do and
> will happen

For remote connections (through mserver), there is already a version check. If the minor version differs, you get a warning, if the major
versions differ (1.>>9<<.4), the client won't start. So at least for remote connection you get a clue.

> For one thing, looking at a given midas-using executable, how do I tell what version of midas it has inside?

Ther is a function cm_get_version() returning the version. As for the executable, all you can do is a

strings <executable> | grep 1.9

> > For one thing, looking at a given midas-using executable, how do I tell what version of midas it has inside?
> 
> Ther is a function cm_get_version() returning the version. As for the executable, all you can do is a
> 
> strings <executable> | grep 1.9

   A lot of programs have a commandline option, such as "--version", where they return the program version number then exit.  As well as the
program version number, the version number of the midas library it's linked with could also be returned (There can be more than one
libmidas.so on a system and this would show which one was currently being linked)

   Something I would find useful would be for the version number to identify precisely which version you have, i.e. not to have different
versions of midas given the same version number.  I've had problems earlier this year due to midas-1.9.3 changing several times between
January and July, while keeping the same number.  I think if "in-between" versions of midas are to be made available, they should contain a
revision number or date or something in the version number to identify them.

> For remote connections (through mserver), there is already a version check. If the minor version differs, you get a warning, if the major
> versions differ (1.>>9<<.4), the client won't start. So at least for remote connection you get a clue.

   Safety measures like these can sometimes get in the way, if you know what you're doing.  So unless there is absolutely no possibility of
success, I think checks such as this one should be overrideable (by a client option).

Chris

>    A lot of programs have a commandline option, such as "--version", where they return the program version number then exit.  As well as the
> program version number, the version number of the midas library it's linked with could also be returned (There can be more than one
> libmidas.so on a system and this would show which one was currently being linked)

This would solve the versioning problem for midas built from versionned tarballs, and I am considering a similar scheme for midas installed from
RPMs. But what do I do for midas built from CVS?!? K.O.

> The current event size in midas is limited to 512k (MAX_EVENT_SIZE in midas.h)

Yes, 512 kBytes is rather small. For the T2K prototype TPC DAQ, I built and ran
MIDAS with 4 MByte events, and it worked fine.

Now, we have per-buffer tunable size (see message
https://ladd00.triumf.ca/elog/Midas/283) and in the long run, I would prefer the
compiled-in limit to go away: already all memory is allocated dynamically and
the MAX_EVENT_SIZE is only useful as kind of a sanity check against frontend
misconfiguration or against malformed events.

If MAX_EVENT_SIZE goes away, the maximum event size becomes limited by the
largest SysV shared memory segment permitted by Linux (via sysctl kernel.shmmax).

To go beyound the limit on SysV shared memories, on can use mmap() based shared
memory: this is limited by available RAM+swap (and disk space for the
.SYSTEM.SHM file). Current MIDAS system.c has an experimental implementation of
mmap() shared memory, but AFAIK it has not been used in any production system, yet.

K.O.

Hello,

I am using a CAEN v1720 to digitise signal coming from 5 PMTs and I need to extend the read-
out window to 1ms.

Given the sampling frequency of 250 MHz, each event would consist of about 4.78 MB

Accordingly to the documentation I found in:

https://midas.triumf.ca/MidasWiki/index.php/Event_Buffer

 - I modified the value of ODB /Experiment/MAX_EVENT_SIZE to 8 MB (I overestimated it in case 
I will readout all 8 channels of the v1720)
 - I modified the ODB key /Experiment/Buffer Sizes/SYSTEM to 512 MB (which allow to contain 
about 100 events in the buffer)

The max_event_size in the frontend source code is set to 32 MB while the event_buffer size is 
200 times the max_event_size. So I did not modify those values.

When I start a new run, the MIDAS crash and the ODB gets corrupted:

$ odbedit 
[ODBEdit,ERROR] [odb.c:1134:db_open_database,ERROR] Different database format: Shared 
memory is 262148000, program is 3
[ODBEdit,ERROR] [midas.c:2157:cm_connect_experiment1,ERROR] cannot open database
Unexpected error #326

Do you have any idea of what might be the problem?

The same thing happen if I reduce the buffer size to 128 MB.

The computer running MIDAS has 2 Quad CPU @ 2.83GHz and 4 GB RAM.

Thank you in advance for any help!
Alberto

In midas.c there are several places where client last activity time stamps are checked against the 
watchdog timeout and the clients are declared dead if they fail to update their activity time stamps. 
ODB time stamps and data buffer time stamps appear to be handled in a similar manner.

Most checks are done like this:

now = ss_millitime();
if (client->watchdog > 0      <----- check that the watchdog is enabled
    && now > client->last_activity    <---- check for crazy time stamps from the future
    && now - client->last_activity > client->watchdog_timeout)   <--- normal timeout
        remove_client(client);

But in a few places, the extra checks are missing:

now = ss_millitime();
if (now - client->last_activity > client->watchdog_timeout)
        remove_client(client);

Is this an oversight from when additional checks were added?
Should I make all checks read like the first one?

K.O.

> In midas.c there are several places where client last activity time stamps are checked against the 
> watchdog timeout and the clients are declared dead if they fail to update their activity time stamps. 
> ODB time stamps and data buffer time stamps appear to be handled in a similar manner.
> 
> Most checks are done like this:
> 
> now = ss_millitime();
> if (client->watchdog > 0      <----- check that the watchdog is enabled
>     && now > client->last_activity    <---- check for crazy time stamps from the future
>     && now - client->last_activity > client->watchdog_timeout)   <--- normal timeout
>         remove_client(client);
> 
> But in a few places, the extra checks are missing:
> 
> now = ss_millitime();
> if (now - client->last_activity > client->watchdog_timeout)
>         remove_client(client);
> 
> Is this an oversight from when additional checks were added?
> Should I make all checks read like the first one?
> 
> K.O.

This is on purpose. Inside cm_watchdog(), the system check for client->watchdog > 0. If the watchdog 
timeout is zero, the client is not removed. This feature is used if you debug a program. If you come to a 
breakpoint and sit there for a while, you might be declared dead and the application is removed from the 
ODB, meaning that you cannot continue debugging (on the next ODB access the application asserts). This 
can be avoided by setting the watchdog to zero, which is implemented in most applications by supplying 
"-d" on the command line. Now assume you debug a program, so you set the watchdog timeout to zero, but in 
the debugging session you decide to quit. Since the watchdog timeout is zero, you will never be removed 
from the ODB. Therefore, the code inside cm_cleanup() doe NOT check client->watchdog > 0. Therefore, a 
"cleanup" inside odbedit will even remove clients having the timeout set to zero. 

Now there might be more clever ways to accomplish that, but that's how it is implemented right now.

Hello,

I have recently set up a midas-2022-05-c instance and have been trying to configure
it to use the mysql odbc interface. Tables are being created for it but
the logger is issuing errors that some of the column types are incorrect. For example
in the log I see:

14:22:12.689 2023/07/25 [Logger,ERROR] [history_odbc.cxx:1531:hs_define_event,ERROR] Error: History event 'Run transitions': Incompatible data type for tag 'State' type 'UINT32', SQL column 'state' type 'INT UNSIGNED'
14:22:12.689 2023/07/25 [Logger,ERROR] [history_odbc.cxx:1531:hs_define_event,ERROR] Error: History event 'Run transitions': Incompatible data type for tag 'Run number' type 'UINT32', SQL column 'run_number' type 'INT UNSIGNED'

Checking the table in the database I see:

MariaDB [t2kgscND280]> describe run_transitions;
+------------+------------------+------+-----+---------------------+-------------------------------+
| Field      | Type             | Null | Key | Default             | Extra                         |
+------------+------------------+------+-----+---------------------+-------------------------------+
| _t_time    | timestamp        | NO   | MUL | current_timestamp() | on update current_timestamp() |
| _i_time    | int(11)          | NO   | MUL | NULL                |                               |
| state      | int(10) unsigned | YES  |     | NULL                |                               |
| run_number | int(10) unsigned | YES  |     | NULL                |                               |
+------------+------------------+------+-----+---------------------+-------------------------------+
4 rows in set (0.000 sec)

Please note that this is not the only history variable that has this problem. There are multiple variables
for which:

Incompatible data type for tag 'Foo Bar' type 'UINT32', SQL column 'foo_bar' type 'INT UNSIGNED'

Checking history_odbc.cxx, I see:

static const char *sql_type_mysql[] = {
   "xxxINVALIDxxxNULL", // TID_NULL
   "tinyint unsigned",  // TID_UINT8
   "tinyint",           // TID_INT8
   "char",              // TID_CHAR
   "smallint unsigned", // TID_UINT16
   "smallint",          // TID_INT16
   "integer unsigned",  // TID_UINT32
   "integer",           // TID_INT32
   "tinyint",           // TID_BOOL
   "float",             // TID_FLOAT
   "double",            // TID_DOUBLE
   "tinyint unsigned",  // TID_BITFIELD
   "VARCHAR",           // TID_STRING
   "xxxINVALIDxxxARRAY",
   "xxxINVALIDxxxSTRUCT",
   "xxxINVALIDxxxKEY",
   "xxxINVALIDxxxLINK"
};

So it seems that unsigned int should map to UINT32.

The database is:
Server version: 10.5.16-MariaDB MariaDB Server

Please let me know if more information is needed.

Note that the choice of using the odbc interface is because we
plan to import an old database that was created using the odbc interface
with a previous version of midas (yes this is your old friend T2K/ND280).

Regards,

Nick.

Hello,

wanted add few things:

1. I see the same problem for INT32
2. For now I've worked around these problems with https://bitbucket.org/nickhastings/midas/commits/e4776f7511de0647077c8c80d43c17bbfe2184fd
3. I'm using mariadb-connector-odbc-3.1.12-3.el9.x86_64 (System is AlmaLinux 9)

Regards,

Nick.