Re: PgAgent Patch - Mailing list pgadmin-hackers

From Ashesh Vashi
Subject Re: PgAgent Patch
Date
Msg-id CAG7mmozBGEirqEEaRRT4s5KS4GOxd-aFGnnGO61=jpKzTQ2LKw@mail.gmail.com
Whole thread Raw
In response to Re: PgAgent Patch  (Mehmet Emin KARAKAŞ <emin100@gmail.com>)
List pgadmin-hackers
On Fri, Apr 17, 2015 at 7:16 PM, Mehmet Emin KARAKAŞ <emin100@gmail.com> wrote:
Hi Sanket,

I test the patch on windows7 and Ubuntu 14.10. It's working fine.
Thanks for the patch.
I have committed this in the master branch.

Also, thanks to Akshay Joshi, Sanket Mehta for the review and improvisation of the patch.
I have modified a bit before check-in.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


http://www.linkedin.com/in/asheshvashi


2015-04-17 12:54 GMT+03:00 Sanket Mehta <sanket.mehta@enterprisedb.com>:
Hi Mehmet,

I have modified the patch as explained below:

After carefully reviewing the patch, we found that there is a memory leak while error file handling.

after opening the file in write mode using freopen(), we again open the same file in read mode using fopen().
at this time we are just opening a file without any stderr stream getting redirected. So it should be closed after read operation.
Which was not happening before and its a memory leak.

previously we were using the same FILE* which is used in freopen() for opening the file using fopen(), so when we close the handler internally it may have close the stream and thus we were not getting the std error for the 2nd time.

But now I have used another FILE* handler to open that file in read mode using fopen() and closed it after read operation and it works fine.
Errors are getting logged in table every time properly.

Another change is that we std errors should get logged in file every time irrespective of output string is empty or not. so I have removed the condition if(output == wxEmptyString) .

I have attached the new patch here.
Please test and let me know if you think any changes requires.


Regards,
Sanket Mehta
Sr Software engineer
Enterprisedb

On Wed, Apr 15, 2015 at 6:23 PM, Sanket Mehta <sanket.mehta@enterprisedb.com> wrote:
Hi,

I have reviewed the patch.
You are right Mehmet, previously 2nd time stderr was not coming to file.
With this patch that issue is resolved.



Regards,
Sanket Mehta
Sr Software engineer
Enterprisedb

On Wed, Apr 15, 2015 at 3:27 PM, Mehmet Emin KARAKAŞ <emin100@gmail.com> wrote:
Hi,


We are using the patch. But  We make a mistake. Pgagent run the agent first time, everything is perfect. But second batch run , pgagent is not get  stderr message. Then I have reviewed the patch. we are using "freopen" for redirect stderr massage to file. Then we  close the error file. But If close the file, close the stderr message at the same time and I don't any error message form stderror. So, I remove the fclose code for errorfile. Now Every things is ok.
I make to test it on windows 7 and ubuntu 14.10. It is working fine. I attached the patch in this mail.

2015-04-09 10:00 GMT+03:00 Sanket Mehta <sanket.mehta@enterprisedb.com>:
Hi,

I have reviewed the patch.
It is working fine.

Regards,
Sanket Mehta
Sr Software engineer
Enterprisedb

On Mon, Apr 6, 2015 at 7:30 PM, Mehmet Emin KARAKAŞ <emin100@gmail.com> wrote:
Hi,

What is the last situation? I want to remove my repo and continue original repo.

Thanks for everything.


2015-03-23 10:58 GMT+02:00 Ashesh Vashi <ashesh.vashi@enterprisedb.com>:
On Mon, Mar 23, 2015 at 2:26 PM, Mehmet Emin KARAKAŞ <emin100@gmail.com> wrote:
I send the diff in previous mail. The name batch_cron_error_20150323.patch. You need to something diffrent. I don't understand.

Oops.
I missed that.

When I saw the job.cpp, I thought - you sent the whole files only.
Thanks for the patch.

Sanket,

Can you please review the patch?


--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


http://www.linkedin.com/in/asheshvashi



2015-03-23 10:36 GMT+02:00 Ashesh Vashi <ashesh.vashi@enterprisedb.com>:
Thanks Mehmet.
Can you please send the diff (as a patch file) instead of the whole file?

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


http://www.linkedin.com/in/asheshvashi


On Mon, Mar 23, 2015 at 1:54 PM, Mehmet Emin KARAKAŞ <emin100@gmail.com> wrote:
Hi Ashesh,

My test scenario is;

"java -jar xxxx" command.

xxx is not a file. I want to see a error message in pgadmin and pga_jobsteplog table. I am waiting to "Error: Unable to access jarfile xxxx" message in job log output.

I make test the patch in windows 7 and ubuntu 14.10. In windows 7 not problem, perfect working. But nix system (Ubuntu 14.10) debug mode, I just saw the massage in terminal. Then I added to message error file to output variable.

Now It's working perfect windows and nix systems. I attach the new patch in this message.


2015-03-23 1:15 GMT+02:00 Mehmet Emin KARAKAŞ <emin100@gmail.com>:
Hi Ashesh ,
Sorry for late. I have test in today and share the results

2015-03-16 10:32 GMT+02:00 Ashesh Vashi <ashesh.vashi@enterprisedb.com>:
On Mon, Mar 16, 2015 at 1:31 PM, Mehmet Emin KARAKAŞ <emin100@gmail.com> wrote:
Hi,

I have not tried the patch because last week was very run down. But I will try the patch in this week and I will share the result.
Thanks.
I really appreciate.

[NOTE: Please keep the list in loop...]


--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


http://www.linkedin.com/in/asheshvashi


2015-03-16 6:16 GMT+02:00 Ashesh Vashi <ashesh.vashi@enterprisedb.com>:
HI Mehmet,

Do you have time to spend time for this?
Can you please review, and test the patch sent by Akshay?

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


http://www.linkedin.com/in/asheshvashi


On Thu, Mar 5, 2015 at 3:07 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Mehmet

I have added the logic in job.cpp file to read the error message, If batch job is failed. Attached is the patch file and job.cpp with the email.
I haven't tested it because facing some difficulty in linking the project on Windows. Can you please apply the patch and test it at your end, also if you seen some modification please feel free to do it. 

On Wed, Mar 4, 2015 at 4:22 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Sure.

On Wed, Mar 4, 2015 at 4:20 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, Mar 4, 2015 at 10:48 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Dave,

On Wed, Mar 4, 2015 at 4:11 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi,

Mehmet may not be able to work on Windows, so one of us may have to deal with that. Did we not solve this already in PEM btw?

    Yes we have solved it in PEM, I have send the source code in my previous email.

Oh, that was it? Well that code is pretty well tried and tested, so maybe we should just use that. Perhaps you can send Mehmet a patch to test on his build?
 

On Wed, Mar 4, 2015 at 10:16 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Mehmet

I have reviewed the patch, but it is only for nix machine. Can you please provide a patch where it should be fixed for Windows as well. I have implemented the same in one of my project using below logic which works for both windows and nix. You can tried out the below logic

errFile = 'xyz.txt';
FILE *fpError = freopen(errFile.mb_str(), "w", stderr); .... .... // If output is empty then either job did not return any output // or job threw some error into stderr. if (output == wxEmptyString) { // Check script threw some error into stderr if (fpError) { fclose(fpError); fpError = fopen(errFile.mb_str(), "r"); if (fpError) { char buffer [4098]; wxString errorMsg = wxEmptyString; while (!feof(fpError)) { if (fgets(buffer, 4096, fpError) != NULL) errorMsg += wxString(buffer, wxConvLibc); } fclose(fpError); } } }

On Wed, Mar 4, 2015 at 10:41 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Sure

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


http://www.linkedin.com/in/asheshvashi


On Tue, Mar 3, 2015 at 2:01 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh; can you please get this reviewed and applied if it looks OK?

Thanks.

On Tue, Mar 3, 2015 at 12:00 PM, Mehmet Emin KARAKAŞ <emin100@gmail.com> wrote:
> Hi,
> If batch cron is failed, pgagent doesn't receive error messages. We fix it.
> We canalized the stderr to stdout. This patch batch cron receive error
> massages from nix machine. Git Diff Message and job.cpp file is attached.
>
> Thanks.
>
> --
> MEHMET EMİN KARAKAŞ
> Postgresql Global Username: emin100
>
>
>
> diff --git a/job.cpp b/job.cpp
> index 00334ab..694be22 100644
> --- a/job.cpp
> +++ b/job.cpp
> @@ -273,8 +273,11 @@ int Job::Execute()
>                                 // The *nix way.
>                                 FILE *fp_script;
>                                 char buf[4098];
> +
> +                wxString wxSt = wxT("");
> +                wxSt.Printf( _("%s 2>&1"), filename.c_str());
>
> -                               fp_script =
> popen(filename.mb_str(wxConvUTF8), "r");
> +                               fp_script = popen(wxSt.mb_str(wxConvUTF8) ,
> "r");
>                                 if (!fp_script)
>                                 {
>                                         output.Printf(_("Couldn't execute
> script: %s, errno = %d"), filename.c_str(), errno);
>
>
>
>
>
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>



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

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


--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers




--
Akshay Joshi
Principal Software Engineer 


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246



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

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



--
Akshay Joshi
Principal Software Engineer 


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246



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

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



--
Akshay Joshi
Principal Software Engineer 


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246



--
Akshay Joshi
Principal Software Engineer 


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246




--
MEHMET EMİN KARAKAŞ
Yazılım Uzmanı

TURKSAT Uluslararası Uydu ve Kablo TV Operatörü
Konya Yolu 40. Km. 06839 Golbasi / ANKARA - TÜRKİYE
Tel     :  +90 312 615 32 73
Faks  :  +90 312 615 32 77




--
MEHMET EMİN KARAKAŞ
Yazılım Uzmanı

TURKSAT Uluslararası Uydu ve Kablo TV Operatörü
Konya Yolu 40. Km. 06839 Golbasi / ANKARA - TÜRKİYE
Tel     :  +90 312 615 32 73
Faks  :  +90 312 615 32 77



--
MEHMET EMİN KARAKAŞ
Yazılım Uzmanı

TURKSAT Uluslararası Uydu ve Kablo TV Operatörü
Konya Yolu 40. Km. 06839 Golbasi / ANKARA - TÜRKİYE
Tel     :  +90 312 615 32 73
Faks  :  +90 312 615 32 77




--
MEHMET EMİN KARAKAŞ
Yazılım Uzmanı

TURKSAT Uluslararası Uydu ve Kablo TV Operatörü
Konya Yolu 40. Km. 06839 Golbasi / ANKARA - TÜRKİYE
Tel     :  +90 312 615 32 73
Faks  :  +90 312 615 32 77




--
MEHMET EMİN KARAKAŞ
Yazılım Uzmanı

TURKSAT Uluslararası Uydu ve Kablo TV Operatörü
Konya Yolu 40. Km. 06839 Golbasi / ANKARA - TÜRKİYE
Tel     :  +90 312 615 32 73
Faks  :  +90 312 615 32 77




--
MEHMET EMİN KARAKAŞ
Yazılım Uzmanı

TURKSAT Uluslararası Uydu ve Kablo TV Operatörü
Konya Yolu 40. Km. 06839 Golbasi / ANKARA - TÜRKİYE
Tel     :  +90 312 615 32 73
Faks  :  +90 312 615 32 77





--
MEHMET EMİN KARAKAŞ
Yazılım Uzmanı

TURKSAT Uluslararası Uydu ve Kablo TV Operatörü
Konya Yolu 40. Km. 06839 Golbasi / ANKARA - TÜRKİYE
Tel     :  +90 312 615 32 73
Faks  :  +90 312 615 32 77

pgadmin-hackers by date:

Previous
From: Ashesh Vashi
Date:
Subject: pgAgent commit: Save the standarad error too along with the standard ou
Next
From: Ashesh Vashi
Date:
Subject: Re: PGAdmin 4 architecture (Was: [Patch] PGAdmin 4 JSON Handling)