RE: [NeoStats-Devel] Idea's for config/data storage

> Justin Hammond wrote:
> > I got two motivations for proposing something new (but to be 
> > honest, I havn't seen the latest commit from Mark about the 
> > config API)
> 
> Well a quick rundown is that originally....

<snip>
Ok.... 

> As a general thought on the presentation below, when I 
> originally created
> the storage system and the associated frameworks for things 
> like botsettings
> etc, I requested feedback on the system on list but none came. I am
> concerned that it is a little late in the day to suggest what 
> may become a
> complete rewrite of the core systems associated with this 
> change. I am keen
> to get 3.0 into release status as soon as possible so that I 
> can take a
> short rest from intensive development and that we can retire 
> 2.5.x.

Hey, as far as I'm concerned, and experience with previous major releases,
your not going to get a break with 3.0 goes "GOLD"... Hehe.
I don't think anybody is going to complain if you take a break now anyway,
as I see most of your changes are complete. :)

 I was
> under the impression that libevent was the only real major 
> change planned
> from this point and had you not been able to implement it, I 
> would have been
> happy to release 3.0 without it and keep it on the todo list for 3.1. 
> 

To be honest, I'm not real keen to jump into a 3.1 series so quickly once
3.0 is out. 
The core needed attention so hence 3.0, but after 3.0 my focus at least will
shift back to the modules, as that what provides the values to NeoStats....
The core is useless without the modules

Plus, considering the work we are putting into 3.0, I think it's a good idea
to make sure things are "right" now rather than have to take on extra code
to deal with those fringe cases later... (not to say that your
implementation isn't right)


> We already have the libevent branch and a partial language 
> implementation
> that thanks to bdb will not even compile on a wide range of systems to
> resolve along with all the other fix ups and tidy ups that we 
> need plus and
> module improvements as we progress through to release. Unless 
> decisions can
> be made now and implementation begin pretty much immediately 
> and unless you
> are returning to full time development, I think we need to 
> consider leaving
> some of these areas in discussion stage for 3.1 so we do not 
> spend forever
> maintaining 2.5.x and 3.0 in mere alpha. 
> 


I can get back into the development at least a couple of times a week now.
I'm involved in another project at the moment, that's for a customer, but
right now, its on hold as the customer now starts work on their application.
Its going to be months before they are ready to move again. 

I think we can implement the config storage without major changes to the
core (other than maybe a rename of the functions and some logic changes to
say config data use this storage "engine" and real data uses that storage
"engine"

As for the current socket code, right now I'm waiting for some improvements
on curl to be done around the socket code. I want to get rid of the entire
process flow where a Library creates a POLLFDS/readfd/writefd/errorfd,
passes it to the core, does the select/poll and returns. Its just not
efficent. (my NeoStats on production spikes and spins on 100% CPU when OPSB
kicks off the scans as its creating/deleting these (sometimes large) arrays
every loop, and its probably the cause of many people saying NeoStats uses
too much CPU...)

The new code will work like the changes I've made to the adns library, where
we register the FD when the library opens it, keep it statically loaded into
libevent and only change/remove it when the socket is closed, or we are now
interested in a different event... 

Eg, a typical HTTP connect would go along the lines off:

Open Socket -> curl registers fd with libevent for POLLOUT -> POLLOUT
activity called -> CURL code writes data to socket -> update libevent to say
we are now interested in POLLIN -> POLLIN activity called -> curl reads
socket and closes socket -> delete fd from libevent.

In the proposed case above, we only call into libevent 3 times... 

Where as the current implementation would call into libevent once for every
FD on every loop.... Not ideal obviously.... 

A local copy of my OPSB does the above already, as does ADNS... 

As for the lang support, as mentioned, that will be migrated to use your DB
abstraction....


> > 1) I'm worried about backwards compatibility if we change 
> > structures between versions. Before we had keeper in 2.5 (I 
> > admit keeper introduced other
> > problems) we had so much legacy code in there to handle 
> > changes to our orignal structures. It really was a nightmare, 
> > and caused lots of issues when users jumped a few versions at 
> > once. Hence my previous suggestion about some sort of 
> > versioning number for config/data entries. 
> 
> OK. I will remind readers of my previous comments on this issue. 
> 
> Config data does not require versioning since it is simple 
> types so only
> stores and retrieves ints, strings and booleans. Config data 
> can be expired
> by stopping using the name and added to with new names. There 
> are no issues
> with backwards compatibility beyond the obvious reuse of a previously
> expired name which would present the same problem in *ANY* 
> system. Obviously
> a change of type would break the config data but this would 
> occur with *ANY*
> system employed. I cannot think of any scenario where 
> configuration data
> should involved a complex data type.
> 

Ok, point taken... I was making the argument in light of if we implement
something like this, then we have that "option" available to us... 


> No versioning is included in the current system for data 
> since it is alpha
> code and may have a number of changes. I would expect data to 
> become fixed
> around late alpha/beta at which point versioning may have 
> value but there
> are alternatives to using a versioning system. For alpha at 
> least, I am
> happy to require recreation as necessary on each release and 
> I would suggest
> this for each release regardless of the implementation since 
> it promotes
> testing of the entire process from testing creation through 
> to reloading of
> known data on subsequent boots. Data is stored in tables so a 
> major change
> to a structure can be simply managed with an alternative table.
> 

Changing version on data during alpha's is just silly, agreed... These are
throughts that I've had into the future when 3.0 becomes "stable"


> > (but the below 
> > solution we don't need that). The problems we had were that a 
> > lot of times, users had segfaults because we didn't "upgrade" 
> > the data in the files in a clean way between versions. I 
> > spent a lot of time maintaining code for this backwards 
> > compatibility (have a look at a old version of statserv 
> > before we moved to keeper.) With what we have now, it 
> > requires a lot of disipline by us to maintain backwards 
> > compatibility. 
> 
> The problems with statserv that required the maintainance of 
> legacy code
> would not be nearly so great since each data area (servers, channels,
> network, etc) now use their own tables rather than writing 
> out structured
> data in the original manner. Any structure change could use a 
> new table and
> legacy code merely needs to be if getv1, get, write, store in 
> new, delete
> else if get v2 etc. In addition, since data reading now 
> utilises callbacks
> for anything other than single items, any update code merely 
> operates on a
> simple data set rather than having to manage transveral of 
> the records.
> Legacy code is therefore unlikely to prove a headache in the 
> same way and
> such a system may remove the need for versioning in many 
> cases since the
> table itself becomes the version.
> 

I disagree that there is much difference to the current implementation as
there is to the old way in terms of maintaining legacy code. Say StatServ
gets a bunch of new features over 10 releases.... This is then 10 different
getv<x> functions to handle the legacy code... 

The original StatServ saved data for channels, servers and network to
different files, much like the same way we use different tables. 

Additionally, if we start adding new options like SQL backends or people
write their own code to read our gdbm databases raw, then they have to
update the code to read the new database as well.... 
This is more along the lines I see with SQL databases though... Which is
discussed further down....


> Completely new data would involve a new table so would not 
> have any issues
> with maintainance of backward compatibility since it would 
> not exist in
> previous incarnations. 
> 
> My plan for modules such as StatServ is to provide 
> placeholders for any
> information we could utilise but currently do not whether or 
> not they are
> useful in the immediate future. The framework is in place 
> throughout the
> core already, I have just not yet added the fields to 
> StatServ. As such, I
> think it unlikely that we will end up with a situtation where 
> we have a
> conflict requiring a major update to the stored data format. 
> I do however
> appreciate the concern about backward comptibility. It was a 
> stick I was
> often beaten with during 2.5.x :)
> 

StatServ is one example, because it's the only module that most widely uses
data storage...



> > 2) It would be cool to offer alternatives to just gdb/bdb 
> > backends for storage. Eg, put config data in human editable 
> > text files, and data into SQL databases (like SQLITE or 
> > postgres/mysql)
> 
> The system as currently implemented was created in a manner to allow
> development of alternatives to gdbm and bdb. It was your 
> suggestion to use
> bdb but the common problems with it meant an alternative had 
> to be found.
> Gdbm was similar enough to allow bdb to be made to work at a 
> later date in
> the same manner which is why I chose it. The abstract use of 
> database name
> and table name were partly to allow later investigation into 
> SQL systems.
> There was an immediate benefit in speed of storage operations 
> and I think
> this an essential prerequisite for any backend support. 
> 

Agreed... And I think we have good flexibility in the backend database
choices..... And lots of alternatives as well...


> > We could easily put our structures into a single field in a 
> > database if we
> > base64 (or equivelent) encode the entire structure, 
> 
> Our structures are already able to go into a single field 
> without encoding
> the structure since they are of a known length. Why would 
> there be a need to
> encode the data?
> 

interopibility.... 
1) structures are likely to contain NULL chars etc... This means we have to
use BLOB types on SQL... And a example is that SQLite doesn't support blobs
(at least last time I checked). But as mentioned, this base64 encoding would
be dependant on the engines.... But this isn't really my main point I'm
getting at... If we want to utilize other options (the only real reason I
user is going to choose SQL is so he can query the data from some other
application, eg a webapp...)

> > but it 
> > doesn't offer the user any way to query the data in a native way.
> 
> Given the apparent lack of interest in SQLServ, is there 
> sufficient interest
> in querying the data outside of NeoStats? Yes it would be 
> great to do so,
> but is it considered essential at this stage. Also consider 
> the obvious path
> the querying will take. The moment it is available there will 
> be a request
> to write as well as query. This would represent a major change to the
> NeoStats systems since NeoStats expects it's data to be 
> written only by
> itself. Any implementation should be created with this in 
> mind so that we
> either allow for external modification or prohibit it.
> 

SQLServ had interest, the problem was that it was damn difficult to get
working. SQLServ never intended to support write's either... And I never had
a request for it... In fact, if we can implement this, I believe we could
drop SQLServ and the headaches that go along with it (btw, its currently
very broken... Something is messed up in the SQL Parser)


For Config data, I think a external rehash option is esential.... We have a
basic infrastucture there to do it already, and with all your SET work, I
think it would be just a matter of calling the Set processor in the core
again... With the flexibility that the SET option gives (in terms of having
callbacks etc) it should be trivial to update... (we can put some cavets
around this, for instance, if you change StatServ's nickname and call
rehash, its not going to work, its going to require a restart.... But if you
update the ExtAuth Service Roots entry, it can be added straight away..... 

For actual Data.... Write support shouldn't be ever considered. 

There might be cases where we need to re-evaluate if our data is really
data, or config entries (for instance, HostServ entries) and maybe
re-implement that...


> One alternative to consider. There is nothing to stop us registering a
> separate table which includes sufficient information to 
> decode a structured
> data record by say having a secondary system which defines 
> the type and name
> of each structure entry. This would make such support 
> optional since without
> the "descriptor" the query cannot decode the information. 
> 


Yeah, but then we have to consider how the interfaces that users will use to
query that data... If they have a C program, it could be easy, but if they
have Perl/Python/Php, it's a lot of hard work to retrive a encoded entry...
I mean, SQL was never intended to store structures this way... 


> > Anyway, having a look at how we doing some of this already, a 
> > lot of the Config variables could easily be ported to the 
> > below interface, as they are usually not structures, but just 
> > single variables. (BUT, if we did do some tricks with the 
> > bot_setting structures, we could load the entire config in 
> > one call to Get_struct below, rather than individual calls to 
> > the database
> > abstractions)
> 
> Acutally we couldn't and I would never recommend following 
> this path. The
> settings system currently does not force the use of a single 
> structure for
> config data since versatility was one of the aims. The core 
> itself uses
> several different systems for "config" data many of which are 
> not members of
> a structure or the same structure. Also, since the core is 
> now "extendable"
> by a module, the core structure would have to contain 
> additional information
> from the modules in order to support a structured read/write 
> system. I do
> not see how the additional complexity and management and the 
> loss of the
> current versatility would be of any worth.
> 

Point noted for Config Data... 

<snip>

> > The below psudeo code *could* be optional depending on the 
> > backend storage we use. For instance, we might want to retain 
> > the current gdb method of storing data, but if a user selects 
> > to store in a config file or full SQL
> > database, we can turn on the   code below and have separate 
> > fields for each
> > entry in a structure. 
> 
> There is nothing stopping a module choosing to store separate 
> fields for
> each structure field. The reason this is not done at any 
> location currently
> is the huge potential performance hit that keeper 
> demonstrated by storing
> data in this manner. IIRC, you have test data which prompted 
> the move to BDB
> for StatServ in 2.5.x. I think this data must be tried with 
> such a system
> before we can consider it for deployment and we should not rely on
> progressive write to work around it since there is a chance 
> that we lose
> large amounts of data unecessarily.
> 

Well, in the other implemention, where its currently being used in a semi
beta enviroment for one of my customers (MQServer was written for one of my
Hosting Clients that supplies news feeds to their clients), the entire
"Client" (it's the one that does the struct encodes/decodes) can process
around 900 messages a second (that's encoding/decoding structures that have
about 30 "fields", and includes the latency in network communications and
"lag" on the server... )

And if its implemented in a way that's smart (ie, lets not update each field
in the SQL Database as each field is processed, but rather save the updates
for one SQL statement (update table (one, two, three) values ("one", "two",
"three");)

The overhead we had with StatServ was keeper's issue, because it opened,
read, and wrote the entire "data" file for each update to a individual
field... Ie, there was no way to do it as a transaction.


> > One of the reasons I also thought about this was that I was 
> > planning to replace the config file we are using now with a 
> > bind style config file. 
> 
> I considered such a change a while ago and decided against it 
> since it is
> more complex for users initially, particularly those familiar with the
> current system, while being overkill given the fact I kept 
> reducing the
> config file to a bare minimum required to boot the system. If 
> we are going
> to follow this path, I think we should implement it 
> immediately since it is
> one area that I had considered stable. 
> 

Porting libconfuse to replace what we have right now should be simple...
Plus I think anything it is a lot simpler that your standard (Unreal
excluded) ircd config file... If they can master your I, O, C/N lines...
Then they should easily pick up anything else we can throw at them!


> > This is a good library called 
> > libconfuse that can handle that style nicely, as well as 
> > providing syntax checking etc. Reading data from confuse is 
> > easy, and it also has the ability to write updates back to 
> > the config file. So if we provide separate "config" files for 
> > each module. Obviously the problems we had with keeper were 
> > related to us using it for data storage as well, so we can 
> > discourage this option for data variables. 
> 
> I have no real objections to this and wish it had been the 
> answer 2 years or
> so ago when keeper was first deployed and config files removed. 
> 
> My main concerns are:
> 
> 1) The addition of yet another library to NeoStats.

The confuse library is actually one two files and one header... I'd just
drop the files direct into the source directory rather than set it up as a
new lib in the /lib directories... 


> 2) After spending much time training users that they did not 
> need to worry
> about config files, we are looking to reintroduce them so we need to
> consider the potential additional support. I think the bootup 
> config must
> remain separate to any "run time" config storage so that 
> users can continue
> to ignore the system if they so desire.

Fully agree. Keep neostats.conf as the main file, and put each modules
config file into the data directory... 




> 3) The current 3.0 config system has neostats create config 
> entries where
> they do not exist at boot time and knows all defaults. It can thus
> automatically update the local config when new items are added while
> maintaining compatibility with existing config data. If we 
> are to distribute
> config files we have to maintain both them and the run time 
> defaults or come
> up with a system where we can update the config file between 
> updates thereby
> only maintaing one set of defaults. Look at 2.5.x modules to 
> see that there
> are various different default settings in that system which 
> only provided
> run time support for defaults. If we do not distribute config 
> files, we lose
> one of the biggest advantages of them, that of offline 
> configuration prior
> to runtime. 

This is why I like confuse... It can write config files out as well... (but
from memory, any comments in existing conf files gets lost...)


> 4) User readable config files suggest the need to support some sort of
> rehash function which will require NeoStats to accept changes to
> configuration outside of the runtime. This relates to the 
> possibility of
> external writing to NeoStats data mentioned above.
> 

Have the rehash signal call the ModuleConfig function, maybe with a new
flag, SET_REHASH... 


> If we go with this system, I say we provide this as the sole means of
> configuration and give no options for storing configuration 
> data in another
> format and it should be implemented immediately. I see no benefit in
> allowing multiple system for storing configuration data since 
> it will be a
> lot more work for us to maintain and support. 
> 

Well, if we write a engine for your current DBA abstraction, a initial
implementation should be able do it without any changes at all to the
core.... Ie, still use DBAFetchConfig* and DBAStoreConfig*, then the only
update that we need to do there is make it a "optional" option to specify a
alternative config engine... And if none is specified, then use the Default
one we already have (gdbm)

The other change we should make is have the DBM option as a "optional" item
now. Set the default to GDBM, and only change if its specified in the
config, which further reduces the number of options users need to choose to
get up and running (only hardcore junkies are really going to have a
preference over GDBM or BDB)


> > Have a look at the code below before reading on.....
> > 
> > If we do some tricks in get_data (or save_data) when using 
> > SQL, like using the name field as the record index, and 
> > retrive the entire record (all fields, not just the 
> > "mystruct[i].name" field) and cache it, we would could make 
> > this pretty fast. (eg, in the first call to get_data, we 
> > cache the entire record, so subsequent calls to get_data just 
> > retrive out of the cache rather than make another SQL call) 
> > (in save_data, we could just have a commit_data call, that 
> > actually sends the SQL update to the database).
> 
> As far as the current code is concerned, all such things are 
> the realm of
> the DBM module so if it can be done, it should not put too 
> much overhead on
> module code or coders. 
> 

Agreed, as mentioned, if the user specifies, GDBM, then this code isn't
used.
If they specify a DB such as SQL, then this code kicks into gear... The
changes to the core is that the Get/SetData functions now take one
additional argument, and that is the structure "descriptor"


> > The code below is actually inspired by the RTA library code 
> > (though very much simplified). I've got a working copy of it 
> > at http://svn.neostats.net/cgi-bin/viewcvs.cgi/MQServer/Trunk 
> > (have a look in Client/testclient.c, Libs/encode_packet.c 
> > (pck_send_message_struct) or Libs/packet.c (pck_decode_message)
> 
> One of the biggest flaws in RTA is the fact is cannot handle 
> some simple
> types through some sort of common callback. For example, the 
> PSTR requiring
> a callback. There is no reason whatsoever that a common 
> routine could not
> handle a simple pointer. You have a length field, you have 
> the pointer.
> Moduleconfig gets it's versatility by only using pointers. 
> This all needs to
> be handled transparently in the core rather than modules 
> having to write
> callbacks for handling such things. 
> 

Well, in fact, our Data should *NEVER* use a pointer with the current
implementation, otherwise when we save data, we are saving the location the
pointer "points" to, rather than the data itself... When we load that
"pointer" data again, it might now be pointing to some other memory location
that doesn't contain anything valid. 

In the MQServer implementation, although its named PSTR (pointer string) its
not intended for that in the final implementation... For instance, in my
other implementation, this is actually intended for graphic data... Which is
going to contain NULL's etc.. And would need to be encoded in some form...
But as that MQServer implementation isn't at that stage yet, I have
implemented something simple. 



> Assuming we are keeping RTA, maybe we would be better aiming 
> to integrate
> RTA with this idea so that we can use a single system for 
> both. This would
> extend the reach of RTA as well since there would be format 
> information
> available to it for far more data. 
> 

Well, if we implement something like this, then lets drop RTA... And for the
user/server/channels data that RTA currently provides, we would write a
"thales" type module to save the data every X minutes etc... And move all
this into a module as your tried to do once... :)


> Anyway, I think we need to make a quick decision on this area since it
> affects more than the data storage subsystems.  An alternative config
> storage layer would more or less drop since I used macros to 
> manage the
> config system to abstract them from the data storage. By 
> changing these to
> functions we should be able to change config data easily 
> without a huge
> impact on the code base. The other ideas however would likely require
> extensive reworking of current systems.
> 

I could drop confuse in very quickly... As mentioned, I'm included to
implement confuse as a "engine" for the DBAConfig* functions rather than
repoint all these macros' at confuse directly... 

Finally, in order to get the BDB system operational, do you mind giving me
access to your system so I can write some configure checks etc... I've
looked over the subversion code that uses BDB heavily, and found they have
checks to determine what version of BDB your using, and thus code
apropriately... :)



> Mark.
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: devel-unsubscribe-wool9L35kifE9wlyV4mCnKxOck334EZe@public.gmane.org
> For additional commands, e-mail: [email protected]
> 
>