Thread: PgAgent 3.3.0 batch scripts on windows always get status failed

PgAgent 3.3.0 batch scripts on windows always get status failed

From
Bastiaan Olij
Date:
Hi All,

First post on this list so I hope I'm doing things right.
We've had a problem for awhile now that any batch script run by pgagent
on windows gets the status failed even if the batch file runs just fine.

I downloaded the latest copy of the source code to have a look and found
something strange in the code for Job::Execute in job.cpp

At the start of this method the variable succeeded gets set to false, in
the windows code it never gets set to true as apposed to the *nix code
where the variable rc is loaded with the return status of the batch
script and succeeded is assigned true if this status equals 0.

In the windows code (line 268) rc is set to 1, the return status of the
batch script is not returned. I do not know if this is done on purpose
and that as we can't detect if the script was successful succeeded is
left as failed or if not setting succeeded to true has simply been
overlooked?

I would suggest changing this code to:
268: // assume successful execution of script
269: rc = 0;
270: succeeded = true;

Alternatively, I'm assuming the code for CloseHandle is calling pclose,
the return value of pclose is the errorlevel returned by the batch
script and could be assigned to rc. I couldn't find the code for
CloseHandle so I'm not sure if it would be able to return this code but
if so the code could be turned into:
267: rc = CloseHandle(h_script);
268: if (rc == 0)
269:    succeeded = true;

Cheers,

Bastiaan Olij




Re: PgAgent 3.3.0 batch scripts on windows always get status failed

From
Dave Page
Date:
Hi

On Thu, Jan 31, 2013 at 2:03 AM, Bastiaan Olij <bastiaan@basenlily.me> wrote:
> Hi All,
>
> First post on this list so I hope I'm doing things right.
> We've had a problem for awhile now that any batch script run by pgagent
> on windows gets the status failed even if the batch file runs just fine.
>
> I downloaded the latest copy of the source code to have a look and found
> something strange in the code for Job::Execute in job.cpp
>
> At the start of this method the variable succeeded gets set to false, in
> the windows code it never gets set to true as apposed to the *nix code
> where the variable rc is loaded with the return status of the batch
> script and succeeded is assigned true if this status equals 0.
>
> In the windows code (line 268) rc is set to 1, the return status of the
> batch script is not returned. I do not know if this is done on purpose
> and that as we can't detect if the script was successful succeeded is
> left as failed or if not setting succeeded to true has simply been
> overlooked?
>
> I would suggest changing this code to:
> 268: // assume successful execution of script
> 269: rc = 0;
> 270: succeeded = true;
>
> Alternatively, I'm assuming the code for CloseHandle is calling pclose,
> the return value of pclose is the errorlevel returned by the batch
> script and could be assigned to rc. I couldn't find the code for
> CloseHandle so I'm not sure if it would be able to return this code but
> if so the code could be turned into:
> 267: rc = CloseHandle(h_script);
> 268: if (rc == 0)
> 269:    succeeded = true;

This seems to be a previously reported issue that slipped through the
net due to lack of feedback. The patch I proposed looks like:

raptor:pgagent dpage$ git diff
diff --git a/job.cpp b/job.cpp
index a9c0c1b..937bc9c 100644
--- a/job.cpp
+++ b/job.cpp
@@ -264,8 +264,8 @@ int Job::Execute()                               }


+                               GetExitCodeProcess(h_script, (LPDWORD)&rc);
CloseHandle(h_script);
-                               rc = 1;
#else                               // The *nix way.

Are you in a position to test it if I build a binary for Windows?

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: PgAgent 3.3.0 batch scripts on windows always get status failed

From
Bastiaan Olij
Date:
Hi Dave,

I don't think that will work, yes it will get the proper rc value but it
still leaves succeeded set to false. It needs inclusion of a check to
see if rc==0 and if so set succeeded to true provided that a successful
script indeed returns an error level of 0 (which I think it should).

And yes, if you could send me a new binary then we are able to test this
swiftly. We do currently use the enterprisedb distributable so I'm
taking the assumption I'll be able to just drop the new .exe in place of
the old for this.

Kindest Regards,

Bastiaan Olij

> This seems to be a previously reported issue that slipped through the
> net due to lack of feedback. The patch I proposed looks like:
>
> raptor:pgagent dpage$ git diff
> diff --git a/job.cpp b/job.cpp
> index a9c0c1b..937bc9c 100644
> --- a/job.cpp
> +++ b/job.cpp
> @@ -264,8 +264,8 @@ int Job::Execute()
>                                 }
>
>
> +                               GetExitCodeProcess(h_script, (LPDWORD)&rc);
>                                 CloseHandle(h_script);
> -                               rc = 1;
>
>  #else
>                                 // The *nix way.
>
> Are you in a position to test it if I build a binary for Windows?
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>




Re: PgAgent 3.3.0 batch scripts on windows always get status failed

From
Dave Page
Date:
Hi

On Mon, Feb 4, 2013 at 11:23 PM, Bastiaan Olij <bastiaan@basenlily.me> wrote:
> Hi Dave,
>
> I don't think that will work, yes it will get the proper rc value but it
> still leaves succeeded set to false. It needs inclusion of a check to
> see if rc==0 and if so set succeeded to true provided that a successful
> script indeed returns an error level of 0 (which I think it should).

Yeah, of course it is somewhat more complex than that. I've attached a
patch thats seems to work for me.

> And yes, if you could send me a new binary then we are able to test this
> swiftly. We do currently use the enterprisedb distributable so I'm
> taking the assumption I'll be able to just drop the new .exe in place of
> the old for this.

Thanks - I'll send you a link to an updated executable off-list.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment

Re: PgAgent 3.3.0 batch scripts on windows always get status failed

From
Dave Page
Date:
[re-adding the list]

On Wed, Feb 6, 2013 at 10:41 PM, Bastiaan Olij <bastiaan@basenlily.me> wrote:
> Hey Dave,
>
> As you've read from David's reply the job's status now comes back as
> successful which solves our immediate issue. I did a little more testing
> to see if I could get it to react to different exit codes and that seems
> to still be ignored so the GetExitCodeProcess command doesn't seem to do
> it's thing (or I'm doing something wrong).

It worked for me :-)

> I simply added an EXIT /B 2 at the end of the script to see if it would
> pick up the error code 2 but the error code returned remained 0.

Remove the /B and it should work. GetExitCodeProcess is returning the
cmd.exe return value, not the scripts ERRORLEVEL.

> Mind you, reading the documentation on GetExitCodeProcess it doesn't
> return an error code if the process hasn't finished running yet and it
> also mentions it's only supported on desktop applications and may thus
> not be available to a service.

I just tested it as a service and it worked fine :-). Proof attached!
(ids 32 and above are run as a service, with pgAgent running under the
LocalSystem account). Ignore some of the earlier ones - they were me
debugging.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment

Re: PgAgent 3.3.0 batch scripts on windows always get status failed

From
"Paragon Corporation"
Date:
We're  having the same issue as described in this post.

http://www.postgresql.org/message-id/CA+OCxoxoCGc7d7tRdyC5-32dDMtXyT1m59s_Lk
3moHTXxfMvmw@mail.gmail.com


It sounds from the post that this issue was fixed.  Was this change ever
pushed to stack builder?

I was thinking maybe it wasn't since the latest source release of pgAdmin

http://www.postgresql.org/ftp/pgadmin3/release/pgagent/

Is before this change was put into place and the pgAgent.exe packaged with
stackbuilder reads File version 3.3.0.0, Product Version 3.3.0

Thanks,
Regina and Leo








Re: PgAgent 3.3.0 batch scripts on windows always get status failed

From
Dave Page
Date:
On Mon, Feb 10, 2014 at 4:58 AM, Paragon Corporation <lr@pcorp.us> wrote:
> We're  having the same issue as described in this post.
>
> http://www.postgresql.org/message-id/CA+OCxoxoCGc7d7tRdyC5-32dDMtXyT1m59s_Lk
> 3moHTXxfMvmw@mail.gmail.com
>
>
> It sounds from the post that this issue was fixed.  Was this change ever
> pushed to stack builder?
>
> I was thinking maybe it wasn't since the latest source release of pgAdmin
>
> http://www.postgresql.org/ftp/pgadmin3/release/pgagent/
>
> Is before this change was put into place and the pgAgent.exe packaged with
> stackbuilder reads File version 3.3.0.0, Product Version 3.3.0

No, there hasn't been a release of pgAgent since this fix was committed.

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: PgAgent 3.3.0 batch scripts on windows always get status failed

From
Bastiaan Olij
Date:
Hi Dave,

I think I speak for a number of people out there, seeing that over the
past year I've received a few emails requesting for the patched version
we have, that it would be nice if a new official release with the fixes
from the past year would see the light of day.

Cheers,

Bas

On 10/02/14 10:37 PM, Dave Page wrote:
> No, there hasn't been a release of pgAgent since this fix was committed. 




Re: PgAgent 3.3.0 batch scripts on windows always get status failed

From
Dave Page
Date:
On Thu, Feb 13, 2014 at 4:45 AM, Bastiaan Olij <bastiaan@basenlily.me> wrote:
> Hi Dave,
>
> I think I speak for a number of people out there, seeing that over the
> past year I've received a few emails requesting for the patched version
> we have, that it would be nice if a new official release with the fixes
> from the past year would see the light of day.

Hi

For the record, that is the only fix in there. Once I've figured out
the versioning with David, I'll look at doing a 3.4.0 release.

> On 10/02/14 10:37 PM, Dave Page wrote:
>> No, there hasn't been a release of pgAgent since this fix was committed.
>
>
>
> --
> Sent via pgadmin-support mailing list (pgadmin-support@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-support



-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company