Merge lp:~jontai/openvista-gtm-integration/bug446635 into lp:openvista-gtm-integration

[Jon Tai]

[jeff.apple (jeff-apple)]

I don't know what routine DDS does or when it's called, so I can't say much. Joel would have to comment.
I was hoping that instead of putting this information in ^TMP, we would put it in ^XTMP with an expiration data of, say, an hour. Then it would get cleared up by the ^XTMP cleanup daemon. ^XTMP is made specifically for these kinds of things. Perhaps that's beyond the scope of what we want right now, though.

review: Abstain

[lp:~jontai/openvista-gtm-integration/bug446635]

98. By Jon Tai on 2009-10-29

clean up ^TMP("MSCZJOB") in GETJOB^MSCZJOBU -- killing just one PID's subscript is not enough

99. By Jon Tai on 2009-10-29

move cleanup logic into JOBEXAM; only clean up data for processes that no longer exist

[Jon Tai (jontai)]

> I don't know what routine DDS does or when it's called, so I can't say much.
> Joel would have to comment.
> I was hoping that instead of putting this information in ^TMP, we would put it
> in ^XTMP with an expiration data of, say, an hour. Then it would get cleared
> up by the ^XTMP cleanup daemon. ^XTMP is made specifically for these kinds of
> things. Perhaps that's beyond the scope of what we want right now, though.

DDS is the screenman routine, AFAIK. Recall that MSCZJOB is the screenman frontend, MSCZJOBU are the utilities, and MSCZJOBS is the %SS clone. ^TMP("MSCZJOB") is where the processes dump their data to. This data is aggregated into ^TMP("MSCZJOB1") by JOBEXAM^MSCZJOBU (actually the real work is done by GETJOB^MSCZJOBU).

After looking at and experimenting with Joel's fix, it does clean up ^TMP("MSCZJOB1"), but it doesn't adequately clean up ^TMP("MSCZJOB"). I fixed this in the latest revision of the branch by moving the cleanup of ^TMP("MSCZJOB") into GETJOB^MSCZJOBU, but the problem with this fix is that it can be too aggressive -- if another process just ran INTRPT^MSCZJOBU but hasn't had a chance to retrieve the results with JOBEXAM^MSCZJOBU yet, the data will be gone. (Moving the data into ^XTMP and relying on an external cleanup routine would suffer from the same race, although rarely).

To solve the race condition, I've moved the cleanup code for ^TMP("MSCZJOB") one more time, this time into JOBEXAM^MSCZJOBU. It only cleans up a process' data if that process no longer exists, so there should never be a race. On the downside, there will always be a little bit of data left in ^TMP("MSCZJOB"), but it will not accumulate forever.

[lp:~jontai/openvista-gtm-integration/bug446635]

100. By Jon Tai on 2009-10-29

update 1st line of routines

[jeff.apple (jeff-apple)]

> DDS is the screenman routine, AFAIK. Recall that MSCZJOB is the screenman
> frontend, MSCZJOBU are the utilities, and MSCZJOBS is the %SS clone.
> ^TMP("MSCZJOB") is where the processes dump their data to. This data is
> aggregated into ^TMP("MSCZJOB1") by JOBEXAM^MSCZJOBU (actually the real work
> is done by GETJOB^MSCZJOBU).
>
> After looking at and experimenting with Joel's fix, it does clean up
> ^TMP("MSCZJOB1"), but it doesn't adequately clean up ^TMP("MSCZJOB"). I fixed
> this in the latest revision of the branch by moving the cleanup of
> ^TMP("MSCZJOB") into GETJOB^MSCZJOBU, but the problem with this fix is that it
> can be too aggressive -- if another process just ran INTRPT^MSCZJOBU but
> hasn't had a chance to retrieve the results with JOBEXAM^MSCZJOBU yet, the
> data will be gone. (Moving the data into ^XTMP and relying on an external
> cleanup routine would suffer from the same race, although rarely).
>
> To solve the race condition, I've moved the cleanup code for ^TMP("MSCZJOB")
> one more time, this time into JOBEXAM^MSCZJOBU. It only cleans up a process'
> data if that process no longer exists, so there should never be a race. On
> the downside, there will always be a little bit of data left in
> ^TMP("MSCZJOB"), but it will not accumulate forever.

I don't see anything wrong with this.

By the way ^XTMP is required to have an expiration (ie cleanup) date. Setting that ahead an hour would keep the cleanup task from removing it too early in an realistic case.

review: Approve

=== modified file 'mumps/MSCZJOB.m'
--- mumps/MSCZJOB.m	2009-06-26 21:06:41 +0000
+++ mumps/MSCZJOB.m	2009-10-29 18:19:10 +0000
@@ -1,10 +1,10 @@
-MSCZJOB	;GFT,JDS/MSC;26JUN2009
+MSCZJOB	;GFT,JDS,JKT/MSC;29OCT2009
 	;;8.0;KERNEL;**MSC**
 	W !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 	N MSC
 DDS	;
 	S DDSFILE=3.081,DR="[MSCZJOBEXAM]",DDSPARM="S"
-	D ^DDS Q
+	D ^DDS K ^TMP("MSCZJOB1",$J) Q
 	;
 UNLOCK(D0)	;FROM FIELD 2, PAGE 3: UNLOCK THE LOCK
 	N X,R,N S R=$G(@MSC@(MSCJOBID,"L",D0)) I R'["^" Q  ;CAN'T SEE IT
 
=== modified file 'mumps/MSCZJOBU.m'
--- mumps/MSCZJOBU.m	2009-06-26 22:50:57 +0000
+++ mumps/MSCZJOBU.m	2009-10-29 18:19:10 +0000
@@ -1,4 +1,4 @@
-MSCZJOBU	;RHL,JDS,JKT/MSC;26JUN2009
+MSCZJOBU	;RHL,JDS,JKT/MSC;29OCT2009
 	;;8.0;KERNEL;**MSC**
 	;
 	;  JOB EXAM UTILITIES FOR GT.M
@@ -31,13 +31,22 @@
 	. N %ZG,%ZRO D NEWZGZRO^ZCD(INSTANCE)
 	. S Y("B",INSTANCE)=%ZG
 	;
+	; get a list of all mumps processes
+	N PIDS D PIDS(.PIDS)
+	N PID S PID=""
+	;
+	; clean up data in ^TMP("MSCZJOB") for processes that no longer exist
+	F  S INSTANCE=$O(Y("B",INSTANCE)) Q:INSTANCE=""  D
+	. F  S PID=$O(^|Y("B",INSTANCE)|TMP("MSCZJOB",PID)) Q:PID=""  D
+	. . I '$D(PIDS(PID)) K ^|Y("B",INSTANCE)|TMP("MSCZJOB",PID)
+	;
+	; consolidate data from ^TMP("MSCZJOB") into XARY
 	I $G(ONEPID) D GETJOB(ONEPID) Q
-	; for each mumps process, search each OpenVista instance for the latest job exam data
-	D PIDS(.XARY) ; XARY is doing double-duty; here it's an array
-	N PID S PID=""
-	F  S PID=$O(XARY(PID)) Q:PID=""  D GETJOB(PID)
+	F  S PID=$O(PIDS(PID)) Q:PID=""  D GETJOB(PID)
 	Q
-GETJOB(PID)	;
+GETJOB(PID)	; private, to be called from JOBEXAM only
+	; search each OpenVista instance for the latest job exam data 
+	; for PID and merge it into XARY
 	N SORTDATE
 	F  S INSTANCE=$O(Y("B",INSTANCE)) Q:INSTANCE=""  D
 	. N H S H=$G(^|Y("B",INSTANCE)|TMP("MSCZJOB",PID,0)) Q:H=""