1
sudhaker
Raising XoopsSessionHandler issue again
  • 2004/3/9 14:24

  • sudhaker

  • Not too shy to talk

  • Posts: 117

  • Since: 2003/2/6 2



Ref:https://xoops.org/modules/newbb/viewtopic.php?topic_id=13891&forum=8#forumpost55849

Inside the method write() of class XoopsSessionHandler, a QUERY decides what to run next, INSERT or UPDATE. So there is always 2 SQL query per session write.

As INSERT will happen just once per session, so trying to UPDATE first and if it fails then doing INSERT can be more efficient way of handling the same.

This will save 1 SQL query for any further request.

I tried it on SQLite with this code, working fine.

$sql = sprintf("UPDATE %s SET sess_updated = %u, sess_data = '%s' WHERE sess_id = '%s'", 'session', time(), $sess_data, $sess_id);
//echo "Debug: $sql <br>";
sqlite_query($this->db, $sql);
if (sqlite_changes($this->db) != 1)
{
$sql = sprintf("INSERT INTO %s (sess_id, sess_updated, sess_ip, sess_data) VALUES ('%s', %u, '%s', '%s')", 'session', $sess_id, time(), $_SERVER['REMOTE_ADDR'], $sess_data);
//echo "Debug: $sql <br>";
sqlite_query($this->db, $sql);
}
return true;

2
Herko
Re: Raising XoopsSessionHandler issue again
  • 2004/3/9 14:46

  • Herko

  • XOOPS is my life!

  • Posts: 4238

  • Since: 2002/2/4 1


Can you subit this in the SourceForge.net's XOOPS Core Development Patch Tracker, where the devs can review them and add them to the next release? I could add it, but if you do so yourself, you are updated on it's status etc.

Herko

3
sudhaker
Re: Raising XoopsSessionHandler issue again
  • 2004/3/9 15:51

  • sudhaker

  • Not too shy to talk

  • Posts: 117

  • Since: 2003/2/6 2


done

4
Herko
Re: Raising XoopsSessionHandler issue again
  • 2004/3/9 16:17

  • Herko

  • XOOPS is my life!

  • Posts: 4238

  • Since: 2002/2/4 1


Thanks! I saw

Herko

5
Brad
Re: Raising XoopsSessionHandler issue again
  • 2004/3/9 16:26

  • Brad

  • Not too shy to talk

  • Posts: 150

  • Since: 2003/12/4


Might I suggest that next time you have more to add to a thread that's already been started, that you add it to THAT thread as opposed to starting a new one? Especially when you are already going through the trouble of adding a link to the other thread and even more especially when the previous thread was yours to begin with.

It makes it easier for people to search and discover your solution when it's all in a nice single thread.

Good job all the same!

Brad

6
intel352
Re: Raising XoopsSessionHandler issue again
  • 2004/3/9 16:31

  • intel352

  • Module Developer

  • Posts: 824

  • Since: 2003/11/23


hehe, /methinks herko has an auto-responder with the Patch Tracker url for forum posts such as this


7
sudhaker
By this time a baby would have born !!!
  • 2005/1/6 16:11

  • sudhaker

  • Not too shy to talk

  • Posts: 117

  • Since: 2003/2/6 2


Ref:https://sourceforge.net/tracker/index.php?func=detail&aid=912823&group_id=41586&atid=430842

It was bit frustrating not to see the code merged in to 2.0.9.2 - even after more than 9 months of patch request submission. By this time a baby would have born, if.......

My suggested change was not going to break anything FOR SURE and all it was supposed to do is decreasing the number of db query by one for almost every request except the first one for any session.

Anyway, guess for most of the people it only matters if the code works. I wish, XOOPS core team were little more concerned about performance and security. I have also seen demo by GIJOE (good work!!! appreciated). Core team should taken these things seriously.

---

Also check this more than a year old post:https://xoops.org/modules/newbb/viewtopic.php?viewmode=flat&topic_id=13526&forum=20 . The expected reaction from XOOPS team was placing a .htaccess file in their default distribution to disallow all .html files under themes directory which will work great for most of the people.

Cheers,

8
Mithrandir
Re: By this time a baby would have born !!!

Quote:
I wish, XOOPS core team were little more concerned about performance and security

I can understand your frustration, but it is a matter of priorities and resources.
Your change does not seem all that big, but we need to make sure that when we try the first SQL query and it fails, we can safely assume the cause and fire off the second query. However, we need to test it - and other things have had higher priorities.

Saying that we are not concerned enough with performance and security hurts. We have in the past 9 months cut down on query count in many areas. XoopsModuleHandler::getByDirname() now saves the result correctly, XoopsMemberHandler::getUsersByGroup() now only takes a couple of queries instead of 1 per found user and there are several other improvements. Saving one query is just not as important as saving 10, 20 or even more... and, admittedly, I have not paid attention to this patch lately.

I will see if I can find the time for it soon.

9
sudhaker
Re: By this time a baby would have born !!!
  • 2005/1/6 17:09

  • sudhaker

  • Not too shy to talk

  • Posts: 117

  • Since: 2003/2/6 2


Thanks for quick response I was almost in hibernated mode for last more than 6 months. It happens when you had to drive 70 miles one way daily to work and project has very tight dead-line.

I’m finding more free time these days so became visible in community again. I have not gone deep into CVS to track the changes (except for my suggestion), but I surely felt that XOOPS has become faster. THANKS TEAM Also community is flooded with great modules and themes. GOOD WORK

Cheers,

10
sudhaker
Re: By this time a baby would have born !!!
  • 2005/1/6 17:50

  • sudhaker

  • Not too shy to talk

  • Posts: 117

  • Since: 2003/2/6 2


I couldn't see XoopsMemberHandler::getUsersByGroup() in CVS Perhaps not commited yet... But great idea !!!

I had to do batch fetch in similar situation in my last project. Some database has limit on number of criteria you can put in where block and Sybase was having limit of 255.

Surprisingly MySQL doesn’t have any problem with 100 thousands items in IN block. So, changes are even simpler...

$ids = array();
for(
$i=0$i 100000; ++$i) {
  
$ids[] = $i;
}
// Performing SQL query
$query 'SELECT * FROM xoops_users WHERE uid IN ('.join(', '$ids).')';

Login

Who's Online

148 user(s) are online (86 user(s) are browsing Support Forums)


Members: 0


Guests: 148


more...

Donat-O-Meter

Stats
Goal: $100.00
Due Date: Apr 30
Gross Amount: $0.00
Net Balance: $0.00
Left to go: $100.00
Make donations with PayPal!

Latest GitHub Commits