Server Stats considerations

[John Adams]

I've actually never used it, to my own surprise. Or maybe I had and totally forgot. Either way, it's working great so far. I'll let it sit on Dev for a few days before committing.

Lookin good tho. I love these stats, man. =)

[John Adams]

Image, (or LE)

Can you elaborate on what I believe was a fix you put into the code for World::WriteServerStatisticsNeededQueries()?

I think we were using just result = query.RunQuery2() but you had suggested that we should not re-use "query", or we'd end up with some sort of memory leak. What's that all about? Can we release the resource and reuse the same query that way?

The purpose of that function was to eventually do all our MOSTs/Highest calcs on a cycle. If we need a new "query" for each one, maybe we should re-think this function, eh?

Thanks

[Scatman]

Yeah I wasn't sure either. I figured some dynamic memory was being allocated somewhere and never released if the query object was re-assigned...but I'm sure LE or Image has better insight on that since I didn't find a reason why it couldn't be reused.

Edit:
Nvm, I see why.
In the switch for RunQuery2, it dynamically allocates an integer. If you just rewrite it without freeing the memory first, you'll lose the handle on that memory. (I think that's why, anyway)

[LethalEncounter]

Image, (or LE)

Can you elaborate on what I believe was a fix you put into the code for World::WriteServerStatisticsNeededQueries()?

I think we were using just result = query.RunQuery2() but you had suggested that we should not re-use "query", or we'd end up with some sort of memory leak. What's that all about? Can we release the resource and reuse the same query that way?

The purpose of that function was to eventually do all our MOSTs/Highest calcs on a cycle. If we need a new "query" for each one, maybe we should re-think this function, eh?

Thanks

Yah, I put that quote there. Basically here is the deal with the Query objects:

In EQEmu, you used to have a create and destroy your query objects which caused issues with people forgetting to clean up after they ran a simple query. When I was designing EQ2Emu, I decided to make it easier on people and automatically clean up whenever the method was completed. This works very well, but there is one point of failure that is still possible. If you run a select query, it will return a result set which is destroyed whenever the query object is automated cleaned up. If you use the same query object to run two select queries, the reference to the old result set sticks around in memory because the query only deletes the last result set when it is done. There are two things that I can do to solve this:

1. To destroy the first result set if the same query object is used more than once. This will crash the emu if you try to use the original result set after you created the second result set.
2. Use a vector to keep track of all result sets and delete them when they are done. This would create additional overhead for every query object to fix something that is only an inconvenience in a few places in the code.
3. Inform people of the problem so that they don't make the mistake.

2 or 3 are valid options, but I lean towards option 3

[Image]

my only change was a null check on the row to be null, there was a query that can return null

In situations of a new database with no characters, the world server was crashing.

[Scatman]

my only change was a null check on the row to be null, there was a query that can return null

In situations of a new database with no characters, the world server was crashing.

Well you were the one to bring it to my attention that multiple query objects needed to be created so there were no memory leaks, which you were absolutely right about. I should've looked at dbcore and database before doing anything in worlddatabase anyway.

[John Adams]

Ok, thanks LE for the explain. The reason I brought it up is that we (Scat and I) have the need to run multiple, if not dozens, of queries in a timer that calculate all the MOST/Highest scores on a server for Stats. At first I was going to suggest we use the new WriteServerStatsNeededQueries() function (not sure how it got that name, but that's another topic lol)... but if I have to define 1 "query" per actual SELECT, we'll have upwards of 24 of them... and that seemed silly.

Would it be better then to make this update/threaded function call 1 function, with 1 query, with params and a switch to execute different ones? That way, the function is called a dozen times, but it's still 1 query define and one SELECT / UPDATE statement at a time?

I want to continue along the lines of smart, efficient code. So whatever you suggest, or Scat comes up with, is perfect with me.

Image, I thought you were the one that commented about multiple queries, since it was in the same function you fixed. Sorry, my mistake

[LethalEncounter]

Bah, fine I'll make it so that you can use the same Query object

[John Adams]

lol, no that's not what I'm asking for. If it's smarter to just call a function from another, we can do that.

[John Adams]

3. Inform people of the problem so that they don't make the mistake.

And I 100% support this notion. Anyone dabbling into the code should have the brains to follow instructions, or don't bother helping

(including me!!)

[LethalEncounter]

lol, no that's not what I'm asking for. If it's smarter to just call a function from another, we can do that.

Too late, already added the support

[John Adams]

All that hemming and hawing about it, and you "fixed" it in 2 minutes.

Developers. Sheesh.


(thank you)

[LethalEncounter]

Wheee, time to work on this. John/Scat, am I still on track by implementing the cpu/memory tracking features we talked about in this old post as well as having the world server upload the stats to the login server for nice server stat pages? We might be able to even give server admins a limited ability to customize pages for their server.

[John Adams]

I like the notion for sure, but please be sure to make that optional, as some admins may get creeped out by us "collecting info"