> What happens is that at the end of the day (I run an intranet where
there
> are about 200 peak users), around midnight, it still shows 80 or so
users
> online! What do you think could be causing this?
Does the code you run to "report" on the user sessions run the code you
posted first? By this I mean does it run through and clear any old
sessions out before reporting how many are still there?
What the code you posted does (badly) is record a timestamp of when a
new session is started, for each user session (although the CFID is by
no means guaranteed to be unique), and then deleting any over 15
minutes. So every session will remain there for 15 minutes (or until
the first page call that runs the skimmer code after the 15 minute limit
has expired) and then be deleted. If the user is still using the
system, they will simply be added back in the next time they access a
page, with a new 15 minute "count".
Therefore, at the end of the day, if there are no page calls made
between 5:30 and midnight, all sessions will still be held in memory,
because the skimmer hasn't been run. I would say that's why you're
still leeing lots of users on line at midnight.
I'd like to ask what you're actually attempting to do with this code.
If it's to see how many people are using the system within the last 15
minutes and how long they've been using the application for, then this
code certainly doesn't do that, or at most does a very bad job of it.
What you'll need to do is add *two* timestamps to each user -- one for
when they first accessed the system, and one when they last requested a
page. Your "15 minute check" should then use the difference between
these values to determine whether the user has been inactive for 15
minutes.
As for Ray's comment of "but that is some pretty awful code" I
completely agree. I've attempted to go through it somewhat below
code-block by code-block -- not changing any of the existing
functionality, but just cleaning it up -- with explanations of what I've
done and why.
> <cflock timeout="15" scope="APPLICATION" type="EXCLUSIVE">
> <cfif NOT isDefined("Application.UsersInfo")>
> <cfset Application.UsersInfo = StructNew()>
> </cfif>
> </cflock>
Since CFMX (I'm assuming you're using MX?), you don't have to worry
about locking shared scopes for reading only, so there's no need to lock
everything as you have done. Put another <cfif> around so you only lock
when absolutely necessary. Keeping the original <cfif> in there just
makes sure that should two requests happen fairly closely, that only one
initialises the new structure.
<cfif NOT isDefined("Application.UsersInfo")>
<cflock timeout="15" scope="APPLICATION" type="EXCLUSIVE">
<cfif NOT isDefined("Application.UsersInfo")>
<cfset Application.UsersInfo = StructNew()>
</cfif>
</cflock>
</cfif>
> <cflock name="#CreateUUID()#" timeout="15" type="EXCLUSIVE">
> <cfset user_cfid = Evaluate(CFID)>
> <cfset user_time = Now()>
> </cflock>
This is a totally pointless lock. You're not writing to (or even
reading from) any shared scope. Additionally, you're using a randomly
generated lock ID so even if you were writing to a shared scope there's
nothing to stop two requests happening simultaneously, because they are
guaranteed to have different lock IDs!
Secondly, I'm not sure why you're using an evaluate() call where you
are, it doesn't do anything except add another function call. The
user's CFID is available anywhere and won't change so there's no need to
reference that as anything different. The user_time variable is only
actually used once, so why not just replace it with the now() call? In
effect, you don't need this section at all.
> <cflock scope="APPLICATION" type="EXCLUSIVE" timeout="15">
> <cfif NOT StructKeyExists(Application.UsersInfo, user_cfid)>
> <cfset temp = StructInsert(Application.UsersInfo, user_cfid,
user_time)>
> </cfif>
> </cflock>
Again, as in the first section, there's no need to lock for a read, so
add another <cfif> statement around the outside of this section.
Replace the user_cfid with CFID and the user_time with now() as
described above. Finally, you don't have to assign the result of the
structinsert() call -- it just means another variable clogging up memory
during the page request:
<cfif NOT StructKeyExists(Application.UsersInfo, CFID)>
<cflock scope="APPLICATION" type="EXCLUSIVE" timeout="15">
<cfif NOT StructKeyExists(Application.UsersInfo, CFID)>
<cfset StructInsert(Application.UsersInfo, CFID, now())>
</cfif>
</cflock>
</cfif>
> <cflock scope="APPLICATION" type="EXCLUSIVE" timeout="15">
> <cfloop collection="#Application.UsersInfo#" item="itmUser">
> <cfif
> Evaluate(DateDiff("n", StructFind(Application.UsersInfo, itmUser),
Now())) GT 15
> >
> <cfset StructDelete(Application.UsersInfo, itmUser)>
> </cfif>
> </cfloop>
> </cflock>
Now this is the sort of block where everyone's opinions will diverge
somewhat. Some will say lock outside the loop, other will say only lock
when deleting. Only locking when deleting will make the code "cleaner",
but *may* leave you open to race conditions where two instances of the
script are running at the same time and try to read and delete from the
same item in the structure at the same time.
By locking the whole loop, if you have a large number of users and page
requests, you might be asking every script to wait while 200 "users" are
looped through. Obviously it's a fairly trivial number in this case,
but the theory is that you wouldn't do that on something with possibly
1000s of sessions, so why in this case?
Personally, I'd change things a bit and use a named lock inside the
loop, with a <cfif> call inside that to make sure that we're not trying
to read or delete something that isn't there any more. Note I've also
removed the unrequired evaluate() call:
<cfloop collection="#Application.UsersInfo#" item="itmUser">
<cflock name="session-lock-#itmUser#" timeout="15">
<cfif structkeyexists(Application.UsersInfo, itmUser)>
<cfif DateDiff("n", Application.UsersInfo[itmUser)>], Now())
GT 15>
<cfset StructDelete(Application.UsersInfo, itmUser)>)>
</cfif>
</cfif>
</cflock>
</cfloop>
I hope that's some help! Now I'll get back to doing something I'm being
paid to do... ;)
Tim.
--
-------------------------------------------------------
Badpen Tech - CF and web-tech: http://tech.badpen.com/
-------------------------------------------------------
RAWNET LTD - Internet, New Media and ebusiness Gurus.
WE'VE MOVED - for our new address, please visit our
website at http://www.rawnet.com/ or call us any time
on 0800 294 24 24.
-------------------------------------------------------
This message may contain information which is legally
privileged and/or confidential. If you are not the
intended recipient, you are hereby notified that any
unauthorised disclosure, copying, distribution or use
of this information is strictly prohibited. Such
notification notwithstanding, any comments, opinions,
information or conclusions expressed in this message
are those of the originator, not of rawnet limited,
unless otherwise explicitly and independently indicated
by an authorised representative of rawnet limited.
-------------------------------------------------------
[Todays Threads] [This Message] [Subscription] [Fast Unsubscribe] [User Settings] [Donations and Support]
