Re: Roster module with custom MySQL requests

2014-01-07 Thread Sylvain Gugli Guglielmi

Le 07/01/2014 13:22, Tomasz Sterna a écrit :


object-sequence is used mainly for sorting - to keep stanza ordering,
etc.
There is no enforcement on uniqueness, but in cases when you do care on
ordering, these should be unique.
In roster table you should be fine with just incrementing ver.
BTW, you could just write your UPDATE queries setting ver with next
sequence number for the table. It will save you incrementing in code and
keep it in line with how the original module does things.
I can have something like UPDATE `roster-items` SET 
`object-sequence`=`object-sequence`+1 but his break uniqueness in the 
table. I haven't found any easy ways to do something similar with 
LAST_INSERT_ID() or AUTO_INCREMENT in MySQL, hence my question. I think 
I'll stick to that for now.



My bet is that loading user data for packet delivery causes that.
There is an old item on Rob's TODO list, to have two load-user events -
one when user logs in, and other used for delivery, which loads only
necessary user data. But it is still TODO
Ok thanks. If performances really become an issue for me, I may get the 
time to look into that.




One of the reasons we have memory pools in jabberd2.




Just to check : you're talking about the pools in pool.h. For the 
record, group management in the roster do not seem to use them. If I'm 
correct, every time an user changes groups, there are potential 
mallocs/frees. Same goes for _roster_user_load. Maybe at one point it'll 
be better to use pools for roster items too (item, item-name, 
item-groups and its content). I've not yet profiled anything, but this 
may be a way to improve speed when loading user data for packet delivery 
(without having to code 2 load-user events).
Then again, I'll focus on the features I need for now. Perf issues will 
come later.


--
Sylvain Gugli Guglielmi




Re: Roster module with custom MySQL requests

2014-01-06 Thread Sylvain Gugli Guglielmi


Le 07/01/2014 00:59, Tomasz Sterna a écrit :

Dnia 2014-01-06, pon o godzinie 18:07 +0100, Sylvain Guglielmi pisze:

Hello everyone,

To use jabberd2 with my pre-existing contacts database, I started
writing a roster module with customisable MySQL requests (I mailed this
list a while back about it, but I just started actual work). It uses
prepared statements, and config file looks like :

This is really interesting. :-D
It could really accelerate XMPP as the next social media protocol. :-)

Could you consider using libdbi instead of hardwiring to MySQL?
This would make your implementation more reusable.


Well, the code is written and I need to go forward right now. But I'll 
look into it when I get the MySQL version running in production. It 
should not be hard provided libdbi works with prepared statements.



- In order to make simpler custom databases, I wanted to remove the
pkt-type == pkt_S10N_UN / item-ask == 2 mechanism. According to a
comment there is no ask='unsubscribe' in RFC bis anymore . Would
anyone advise against it ?

How would that make schema simpler?
You still need to store ask='subscribe', don't you?
Yes, but I assumed it is simpler to explain in the config file the 4th 
parameter of the statement is a boolean meaning that the user is 
requiring subscription instead of a int meaning either requiring a 
subscription or an unsubscription. Especially if it's not a standard 
feature. It also cleans a few lines of code. But you're right : the 
benefit isn't worth bothering. I won't remove it.



- something that seems weird to me : I was expecting item-ver to be
incremented each time the item is updated (for example in
_roster_save_item),
but couldn't find such code. It there something I don't grasp ?

Ahhh... Yes... There's a little trick I used there. ;-)

ver is coming from auto-incrementing field object-sequence.
As storage never issues UPDATE but always DELETE first then INSERT,
every updated roster item automagically gets new ver value.
Ok, I get it now. My custom requests however will use Update... So I'll 
add the increment when the item is modified. As I understand it, the 
object-sequence don't need to be an UNIQUE field. Am I right on that ?





[...] I thought it would be good to do the same
thing in the regular roster, but I couldn't find a way to remove a
specific os_object_t. Is there a way to do that ?

storage_delete() with proper filter.


Thanks for the insight.




Anyway here's what the code could look like, with TODOs :

The clean separation between stanza parsing to roster-item and
roster-group structures which are then stored to DB, and in reverse
makes the code more comprehensible.

I doubt that trading readability for efficiency is worthy in case of
today's microprocessor speeds and RAM availability.
Well, I have broken that clean separation early on. For example I needed 
to check if the max number of contacts in the roster is reached with a 
MySQL request, etc... So this part of readability is already lost in my 
code :S.
Also, with many more requests, I fear the jabberd2 load will be more 
important when I switch our prod to this plugin, and the load is already 
noticeable (15% daily peak of our server)... It seemed to me a good 
tradeoff, I'll stick to that right now. Anyway it'll be easy to revert 
that later on and test the change in performance..



Also, does your use-case covers users in many-many groups?
My experience shows that most users are in 0 groups, some in 1 group and
very few in 2+ groups. There's not many INSERTs to gain there.

Remember: Premature optimization is the root of all evil.
...until your profiling shows 1000 functions each taking 0.1% of the 
time, then you regret not having been careful when writing each of 
them. ^_^ (I lack insight on what is costly for the SessionManager, but 
in my experience mallocs and frees can be long, so I try to minimize 
these calls as a rule. I assumed the same goes for MySQL requests).

I would rather suggest adding 'dirty' flag, risen whenever group
membership actually changes. This would keep the separation still,
allowing for bulk drops of unneeded DELETE/INSERTs.




Thanks for the feedback. I'll keep everyone updated.

--
Sylvain Gugli Guglielmi