Code review comment for lp:~jontai/openvista-gtm-integration/bug446635

Revision history for this message
jeff.apple (jeff-apple) wrote :

> 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

« Back to merge proposal