Обсуждение: pgAgent: I want to make a port to C++ Only

Hello @All,

i want to make a port away from wxWidgets to a c++ only job scheduler.

Why:

I want to reduce dependencies. A framework like wxWidgets should not installed to server systems without GUI (linux or windows server)

C++ libs are already installed in most cases.

i want to push / support the development.

Questions:

Is this supported from pgAdmin developers?

Exists another point of view for this thema?

Thanks

Thomas Steffen

- Software Developer and engineer for electric technology

- C/C++, Perl, VBA and many more

On Mon, Sep 9, 2013 at 7:49 AM, Linreg <linreg@gmx.net> wrote:
> Hello @All,
>
>
>
> i want to make a port away from wxWidgets to a c++ only job scheduler.
>
>
>
> Why:
>
> I want to reduce dependencies. A framework like wxWidgets should not
> installed to server systems without GUI (linux or windows server)
>
> C++ libs are already installed in most cases.
>
> i want to push / support the development.
>
>
>
> Questions:
>
> Is this supported from pgAdmin developers?
>
> Exists another point of view for this thema?

I would certainly like to see a pure C++ port of pgAgent, but I should
also point out that the current code doesn't require any of the GUI
components of wxWidgets to be installed. The problem is really with
the wxWidgets packagers, many of whom have split their packages into
GUI and non-GUI components :-(


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

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

Am Montag, 9. September 2013, 08:31:11 schrieb Dave Page:

> On Mon, Sep 9, 2013 at 7:49 AM, Linreg <linreg@gmx.net> wrote:

> > Hello @All,

> >

> > i want to make a port away from wxWidgets to a c++ only job scheduler.

> >

> > Why:

> >

> > I want to reduce dependencies. A framework like wxWidgets should not

> > installed to server systems without GUI (linux or windows server)

> >

> > C++ libs are already installed in most cases.

> >

> > i want to push / support the development.

> >

> > Questions:

> >

> > Is this supported from pgAdmin developers?

> >

> > Exists another point of view for this thema?

>

> I would certainly like to see a pure C++ port of pgAgent, but I should

> also point out that the current code doesn't require any of the GUI

> components of wxWidgets to be installed. The problem is really with

> the wxWidgets packagers, many of whom have split their packages into

> GUI and non-GUI components :-(

Hello Dave,

Vision:

a smart migration to C++

# Phase 1

+ stay on base structure (programm flow)

+ backward compatibilty (commandline parameter, sql-schema)

+ remove wxWidget dependencies

+ replace all wx classes with c++ classes (std::string, stringstream, thread etc)

+ logfile has csv structure, that is needed in Phase 2

(like 2013-12-01 12:13|DEBUG|bla bla ...)

+ remove mutex

+ replace connectstring parsing

# Phase 2

+ new impelenting of commandline parsing

+ move pgagent schema in separat database (better access rights...) this need a patch for pgadmin

+ file_fdw configuration to show logfile-entrys over a sql view

This is a 90% pure C++ solution :), because

- The windwos side is more C than C++ like.

- I still would like to avoid dependencies to libraries like boost etc.

Phase 1 is almost finished

Question:

Pure enough? :)

Thanks

Thomas Steffen

Hi

On Mon, Sep 9, 2013 at 2:37 PM, Linreg <linreg@gmx.net> wrote:
> # Phase 1
>
> + stay on base structure (programm flow)
>
> + backward compatibilty (commandline parameter, sql-schema)

That's definitely required.

> + remove wxWidget dependencies
>
> + replace all wx classes with c++ classes (std::string, stringstream, thread
> etc)

Yes.

> + logfile has csv structure, that is needed in Phase 2

Why? Not sure what that has to do with porting to C++?

> (like 2013-12-01 12:13|DEBUG|bla bla ...)
>
> + remove mutex

Err, again, why? The mutex is there for a reason.

> + replace connectstring parsing
>
>
>
> # Phase 2
>
> + new impelenting of commandline parsing
>
> + move pgagent schema in separat database (better access rights...) this
> need a patch for pgadmin

That's very unlikely to be accepted - we intentionally use the
maintenance database for that, rather than forcing the user to use a
different one.

> + file_fdw configuration to show logfile-entrys over a sql view

OK, well that explains the CSV format above, but of course, this will
only work if the agent is known to be on the same host. At present, we
don't know that.

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

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

Hello Dave,

+ move pgagent schema in separat database (better access rights...) this

need a patch for pgadmin

>

> That's very unlikely to be accepted - we intentionally use the

> maintenance database for that, rather than forcing the user to use a

> different one.

>

I regard the postgres database as a system-table-database, not as a maintenance database. Is this a wrong reflection?

And would you prefer user postgres as standard pgAgent-user?

I think an normal user is better. Better for give explicit access rights

My concept is similar to the job scheduler from MS-SQL server.

+ file_fdw configuration to show logfile-entrys over a sql view

>

> OK, well that explains the CSV format above, but of course, this will

> only work if the agent is known to be on the same host. At present, we

> don't know that.

Yes, i understand. Maybe this could be an Addon.

I think, in the most cases the agent and the server are on same host.

okay.

When i have adapted the source code. How and Where do I deploy it.

Thomas Steffen

---------------------------------------------------------

- Software Developer and engineer for electric technology

- C/C++, Perl, VBA and many more

---------------------------------------------------------

On Tue, Sep 10, 2013 at 10:45 AM, Linreg <linreg@gmx.net> wrote:
>> That's very unlikely to be accepted - we intentionally use the
>
>> maintenance database for that, rather than forcing the user to use a
>
>> different one.
>
>>
>
> I regard the postgres database as a system-table-database, not as a
> maintenance database. Is this a wrong reflection?

Yes. It was specifically added for that purpose (I can say that with
authority - I wrote the patch)!

> And would you prefer user postgres as standard pgAgent-user?
>
> I think an normal user is better. Better for give explicit access rights

The current code allows you to use whatever user you like, though
"postgres" is the default. That should not be changed.

> When i have adapted the source code. How and Where do I deploy it.

Send a patch against git head to this list, and one of us will review
it, and provide feedback until it's in a state that we're happy to
commit.

Thanks!

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

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

Hello Dave,

here are my first patches. i hope the format is correct.

a liitle summary of my changes:

-------------------------------

+ linux code is tested from me

+ windows code is edited, but not tested

+ wx-lib removed

+ c++ classes used

+ no mutex needed, database connection is rewritten, less code, simpler codeflow

+ cmake files are not adopted, i dont know how. i use qmake (sorry for that)

+ connectstring parsing rewritten

+ LogMessage has a timestamp column and is in csv like structure

+ pure c++ trim and replace function

+ ip validation: ipv6 testing is to simple in the moment

+ ip validation: unix domain socket alias filepath for host-values are accepted

Question:

can anyone give me a checklist for the window side?

Or has anyone a visualstudio project file?

Thomas Steffen

---------------------------------------------------------

- Software Developer and engineer for electric technology

- C/C++, Perl, VBA and many more

---------------------------------------------------------

Hi

Can you send a single patch please? "git diff > patch.diff" should do that.

Thanks.

On Wed, Sep 11, 2013 at 1:54 PM, Linreg <linreg@gmx.net> wrote:
> Hello Dave,
>
>
>
> here are my first patches. i hope the format is correct.
>
>
>
> a liitle summary of my changes:
>
> -------------------------------
>
> + linux code is tested from me
>
> + windows code is edited, but not tested
>
>
>
> + wx-lib removed
>
> + c++ classes used
>
> + no mutex needed, database connection is rewritten, less code, simpler
> codeflow
>
> + cmake files are not adopted, i dont know how. i use qmake (sorry for that)
>
> + connectstring parsing rewritten
>
> + LogMessage has a timestamp column and is in csv like structure
>
> + pure c++ trim and replace function
>
> + ip validation: ipv6 testing is to simple in the moment
>
> + ip validation: unix domain socket alias filepath for host-values are
> accepted
>
>
>
> Question:
>
> can anyone give me a checklist for the window side?
>
> Or has anyone a visualstudio project file?
>
>
>
> Thomas Steffen
>
> ---------------------------------------------------------
>
> - Software Developer and engineer for electric technology
>
> - C/C++, Perl, VBA and many more
>
> ---------------------------------------------------------



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

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

Hi,

here is it

Thomas Steffen

---------------------------------------------------------

- Software Developer and engineer for electric technology

- C/C++, Perl, VBA and many more

---------------------------------------------------------

Hi

On Wed, Sep 11, 2013 at 2:14 PM, Linreg <linreg@gmx.net> wrote:
> Hi,
>
>
>
> here is it

Thanks. I've taken a brief look, and found a number of initial issues,
and have some questions:

- The new code doesn't match the existing coding style - you've used 4
spaces instead of tabs for indents, and braces to open a new context
are not on a new line for example. Please follow the existing coding
style to make the diffs (much) easier to read. You might also try
something like: astyle -n -p -b -S -t4 -k3 -z2 `find . -name "*.cpp"
-o -name "*.h"`

- There are no updates to the build system, which seems essential for
this patch. As a result, I can't compile the code at all yet.

- There are numerous comments that are clearly notes to yourself, and
lines of code that have been commented out. This needs to be cleaned
up before anything can be committed.

- You seem to have hard-coded the exit code from Windows batch job
steps to 1. Why?

- You also seem to have removed the connection pool. Why?

I think that's it for now. I'll look further when I can build the code :-)

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

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

Hi Dave,

- The new code doesn't match the existing coding style - you've used 4

spaces instead of tabs for indents, and braces to open a new context

are not on a new line for example. Please follow the existing coding

style to make the diffs (much) easier to read. You might also try

something like: astyle -n -p -b -S -t4 -k3 -z2 `find . -name "*.cpp"

-o -name "*.h"`

=> done with astyle

- There are numerous comments that are clearly notes to yourself, and

lines of code that have been commented out. This needs to be cleaned

up before anything can be committed.

=> cleaned up

- There are no updates to the build system, which seems essential for

this patch. As a result, I can't compile the code at all yet.

=> done. wx settings are out and sdtc++ are in.

=> cmake and make running without warnings or errors

=> My Plattform:

64-Bit Linux 3.7.10-1.16

openSuse Distro

gcc 4.7.2 with std=c++11

- You seem to have hard-coded the exit code from Windows batch job

steps to 1. Why?

=> i changed back to git-head version. i believe it come from old 3.3.0 source code version. I will never make such things.

- You also seem to have removed the connection pool. Why?

Why not?

Pos:

+ The code is simpler (no locking at all, fewer code)

+ easier maintenance

Neg:

- i'm not sure. maybe more parallel connection.

There are two scenarios:

normal typ: two or three dozen job. only a few running in parallel

big typ: many many jobs. many running in parallel. This companies have other problems or use a connection pooler like pgpool.

In summary the positive points weigh more heavily

Thomas

Hi

On Fri, Sep 13, 2013 at 11:43 AM, Linreg <linreg@gmx.net> wrote:
> Hi Dave,
>
>
>
> - The new code doesn't match the existing coding style - you've used 4
>
> spaces instead of tabs for indents, and braces to open a new context
>
> are not on a new line for example. Please follow the existing coding
>
> style to make the diffs (much) easier to read. You might also try
>
> something like: astyle -n -p -b -S -t4 -k3 -z2 `find . -name "*.cpp"
>
> -o -name "*.h"`
>
>
>
> => done with astyle

OK.

> - There are numerous comments that are clearly notes to yourself, and
>
> lines of code that have been commented out. This needs to be cleaned
>
> up before anything can be committed.
>
>
>
> => cleaned up

OK.

> - There are no updates to the build system, which seems essential for
>
> this patch. As a result, I can't compile the code at all yet.
>
>
>
> => done. wx settings are out and sdtc++ are in.
>
> => cmake and make running without warnings or errors
>
> => My Plattform:
>
> 64-Bit Linux 3.7.10-1.16
>
> openSuse Distro
>
> gcc 4.7.2 with std=c++11

That's not playing nicely with my compiler:

viper:pgagent dpage$ make all
Scanning dependencies of target pgagent
[ 14%] [ 28%] [ 42%] [ 57%] Building CXX object CMakeFiles/pgagent.dir/job.cpp.o
Building CXX object CMakeFiles/pgagent.dir/connection.cpp.o
Building CXX object CMakeFiles/pgagent.dir/misc.cpp.o
Building CXX object CMakeFiles/pgagent.dir/pgAgent.cpp.o
cc1plus: error: unrecognized command line option "-std=c++11"cc1plus:
error: unrecognized command line option "-std=c++11"

cc1plus: error: unrecognized command line option "-std=c++11"
cc1plus: error: unrecognized command line option "-std=c++11"
make[2]: *** [CMakeFiles/pgagent.dir/connection.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[2]: *** [CMakeFiles/pgagent.dir/misc.cpp.o] Error 1
make[2]: *** [CMakeFiles/pgagent.dir/pgAgent.cpp.o] Error 1
make[2]: *** [CMakeFiles/pgagent.dir/job.cpp.o] Error 1
make[1]: *** [CMakeFiles/pgagent.dir/all] Error 2
make: *** [all] Error 2

That's with i686-apple-darwin11-llvm-g++-4.2 (GCC) 4.2.1 (Based on
Apple Inc. build 5658) (LLVM build 2336.11.00), the default compiler
on Mac OS X Mountain Lion. FYI, we need to support much older
compilers than that, for example, g++ (GCC) 4.1.2 20080704 (Red Hat
4.1.2-54) ships with RHEL 5, which is a supported platform.

> - You seem to have hard-coded the exit code from Windows batch job
>
> steps to 1. Why?
>
>
>
> => i changed back to git-head version. i believe it come from old 3.3.0
> source code version. I will never make such things.

I'm working on git-head (d6c5a807110a7ec6ecde9ad59dd7e3b35891fb5e),
and see this when I apply your patch:

-                               GetExitCodeProcess(h_process, (LPDWORD)&rc);
-                               CloseHandle(h_process);
                                CloseHandle(h_script);
-
+                               rc = 1;

This is clearly changing the logic here, which it should not.

> - You also seem to have removed the connection pool. Why?
>
>
>
> Why not?
>
> Pos:
>
> + The code is simpler (no locking at all, fewer code)
>
> + easier maintenance
>
>
>
> Neg:
>
> - i'm not sure. maybe more parallel connection.
>
>
>
> There are two scenarios:
>
> normal typ: two or three dozen job. only a few running in parallel
>
>
>
> big typ: many many jobs. many running in parallel. This companies have other
> problems or use a connection pooler like pgpool.
>
>
>
> In summary the positive points weigh more heavily

This change is unrelated to porting to pure C++, and needs to be
discussed and (if acceptable) implemented as a separate patch. I'm not
convinced it's an appropriate change at all - I certainly work with
customer who do not use a connection pooler for various reasons, and
rely on the pooler in the agent to prevent large numbers of
connect/disconnect cycles, which amongst other things use resources
unnecessarily, and can fill up audit logs.

Another problem that I've noticed is that you've unconditionally
changed the logging format to be pipe delimited. This is also not
acceptable as part of this patch, and should be discussed and
implemented separately. At minimum, this would need to be a
configurable behaviour change, and by default, I would want it to
implement standard (comma delimited) CSV, not a pipe delimited
version.

Thanks, Dave.

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

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

Hi Dave,

> > - You seem to have hard-coded the exit code from Windows batch job

> >

> > steps to 1. Why?

> >

> >

> >

> > => i changed back to git-head version. i believe it come from old 3.3.0

> > source code version. I will never make such things.

>

> I'm working on git-head (d6c5a807110a7ec6ecde9ad59dd7e3b35891fb5e),

> and see this when I apply your patch:

>

> - GetExitCodeProcess(h_process, (LPDWORD)&rc);

> - CloseHandle(h_process);

> CloseHandle(h_script);

> -

> + rc = 1;

>

> This is clearly changing the logic here, which it should not.

Sorry. Sorry. :(

i corrected this. Done

> Another problem that I've noticed is that you've unconditionally

> changed the logging format to be pipe delimited. This is also not

> acceptable as part of this patch, and should be discussed and

> implemented separately. At minimum, this would need to be a

> configurable behaviour change, and by default, I would want it to

> implement standard (comma delimited) CSV, not a pipe delimited

> version.

okay. i change this feature to configurable behaviour.

By default logging will be as before.

can i add an commandline parameter for this?

is it than acceptable?

> That's not playing nicely with my compiler:

>

> viper:pgagent dpage$ make all

> Scanning dependencies of target pgagent

> [ 14%] [ 28%] [ 42%] [ 57%] Building CXX object

> CMakeFiles/pgagent.dir/job.cpp.o Building CXX object

> CMakeFiles/pgagent.dir/connection.cpp.o

> Building CXX object CMakeFiles/pgagent.dir/misc.cpp.o

> Building CXX object CMakeFiles/pgagent.dir/pgAgent.cpp.o

> cc1plus: error: unrecognized command line option "-std=c++11"cc1plus:

> error: unrecognized command line option "-std=c++11"

>

> cc1plus: error: unrecognized command line option "-std=c++11"

> cc1plus: error: unrecognized command line option "-std=c++11"

> make[2]: *** [CMakeFiles/pgagent.dir/connection.cpp.o] Error 1

> make[2]: *** Waiting for unfinished jobs....

> make[2]: *** [CMakeFiles/pgagent.dir/misc.cpp.o] Error 1

> make[2]: *** [CMakeFiles/pgagent.dir/pgAgent.cpp.o] Error 1

> make[2]: *** [CMakeFiles/pgagent.dir/job.cpp.o] Error 1

> make[1]: *** [CMakeFiles/pgagent.dir/all] Error 2

> make: *** [all] Error 2

>

> That's with i686-apple-darwin11-llvm-g++-4.2 (GCC) 4.2.1 (Based on

> Apple Inc. build 5658) (LLVM build 2336.11.00), the default compiler

> on Mac OS X Mountain Lion. FYI, we need to support much older

> compilers than that, for example, g++ (GCC) 4.1.2 20080704 (Red Hat

> 4.1.2-54) ships with RHEL 5, which is a supported platform.

Okay, thats a problem.

I use features from c++11 standard like "atomic" and "thread"

My suggestion:

the Pure-C++-Version is is for newer OS / Compiler suits and not backward compatible (relating compiler / c++ version)

Is the way a possible?

> > - You also seem to have removed the connection pool. Why?

> >

> >

> >

> > Why not?

> >

> > Pos:

> >

> > + The code is simpler (no locking at all, fewer code)

> >

> > + easier maintenance

> >

> >

> >

> > Neg:

> >

> > - i'm not sure. maybe more parallel connection.

> >

> >

> >

> > There are two scenarios:

> >

> > normal typ: two or three dozen job. only a few running in parallel

> >

> >

> >

> > big typ: many many jobs. many running in parallel. This companies have

> > other

> > problems or use a connection pooler like pgpool.

> >

> >

> >

> > In summary the positive points weigh more heavily

>

> This change is unrelated to porting to pure C++, and needs to be

> discussed and (if acceptable) implemented as a separate patch. I'm not

> convinced it's an appropriate change at all - I certainly work with

> customer who do not use a connection pooler for various reasons, and

> rely on the pooler in the agent to prevent large numbers of

> connect/disconnect cycles, which amongst other things use resources

> unnecessarily, and can fill up audit logs.

it is very time consuming to keep this feature for c++-port.

please tell me for my understanding:

In the moment you reuse a connection object. okay.

But how can you use it for an another host / user combination<ß The same object.

Example

Job 1 - step 1: connect to User: x Host A

Job 1 - step 2: connect to User: x Host B

if you use the same connection object, the database log has a connect/disconnect entry.

Job 1 - step 1: connect to User: x Host A

Job 1 - step 2: connect to User: x Host A

And when the same Host/User combination is for a reused connection:

How make you a logical reset for the database server?

In the last scenario can i make a reusing too. With move the Jobthread-SQL-action to the Mainloop i can reduce the needed connection count to 1xJob count + 1 (service connect). in my implementation i need at the moment 2xJobcount+1 connection objects.

The connect/disconnect is one time for every job that will be running. That can be removed too. (move the Jobthread-SQL-action to the Mainloop)

Thanks and a nice weekend

Thomas

Hi Dave,

>

> This change is unrelated to porting to pure C++, and needs to be

> discussed and (if acceptable) implemented as a separate patch. I'm not

> convinced it's an appropriate change at all - I certainly work with

> customer who do not use a connection pooler for various reasons, and

> rely on the pooler in the agent to prevent large numbers of

> connect/disconnect cycles, which amongst other things use resources

> unnecessarily, and can fill up audit logs.

A little addendum:

- connection pooling per Job is not a problem. This changes send i in the next week.

- connection pooling of all jobs has no effect in your code. The function clearconnection released connection-objects before it can be reused after one mainloop cycle. I think. I may be wrong.

so connection pooling per Job should be enough. What do you mean.

Thomas Steffen

Hi Dave,

Connection Pooling:

okay. I will reimplementing the connection pool.

Am Sonntag, 15. September 2013, 15:02:03 schrieb Dave Page:

> > > Another problem that I've noticed is that you've unconditionally

> > >

> > > changed the logging format to be pipe delimited. This is also not

> > >

> > > acceptable as part of this patch, and should be discussed and

> > >

> > > implemented separately. At minimum, this would need to be a

> > >

> > > configurable behaviour change, and by default, I would want it to

> > >

> > > implement standard (comma delimited) CSV, not a pipe delimited

> > >

> > > version.

> >

> > okay. i change this feature to configurable behaviour.

> >

> > By default logging will be as before.

> >

> > can i add an commandline parameter for this?

> >

> > is it than acceptable?

>

> I think so - but please do that as a separate patch. We don't like to mixup

> multiple changes in the same patch/commit as it's harder to find issues or

> review the history in the future.

okay. I will make it so.

> > Okay, thats a problem.

> >

> > I use features from c++11 standard like "atomic" and "thread"

> >

> > My suggestion:

> >

> > the Pure-C++-Version is is for newer OS / Compiler suits and not backward

> > compatible (relating compiler / c++ version)

> >

> > Is the way a possible?

>

> Not really, as it requires maintenance of 2 code branches until we can

> completely deprecate the old wxWidgets code. That's why PostgreSQL itself

> is extremely conservative about what they'll support. We're not nearly as

> bad as that, but we do need to carry on supporting RHEL 5 and similarly

> aged OSs for a while.

I examine whether the use of the boost lib is possible. it should.

I think they has a better backward compatibility.

Please can you review this for me by http://www.boost.org/development/tests/release/developer/statechart.html

you know better than i the version of supported compiler / plattforms.

Thomas Steffen

On Tue, Sep 17, 2013 at 1:59 PM, Linreg <linreg@gmx.net> wrote:
> Hi Dave,
>
>
>
> Connection Pooling:
>
> okay. I will reimplementing the connection pool.
>
>
>
> Am Sonntag, 15. September 2013, 15:02:03 schrieb Dave Page:
>
>> > > Another problem that I've noticed is that you've unconditionally
>
>> > >
>
>> > > changed the logging format to be pipe delimited. This is also not
>
>> > >
>
>> > > acceptable as part of this patch, and should be discussed and
>
>> > >
>
>> > > implemented separately. At minimum, this would need to be a
>
>> > >
>
>> > > configurable behaviour change, and by default, I would want it to
>
>> > >
>
>> > > implement standard (comma delimited) CSV, not a pipe delimited
>
>> > >
>
>> > > version.
>
>> >
>
>> > okay. i change this feature to configurable behaviour.
>
>> >
>
>> > By default logging will be as before.
>
>> >
>
>> > can i add an commandline parameter for this?
>
>> >
>
>> > is it than acceptable?
>
>>
>
>> I think so - but please do that as a separate patch. We don't like to
>> mixup
>
>> multiple changes in the same patch/commit as it's harder to find issues or
>
>> review the history in the future.
>
>
>
> okay. I will make it so.
>
>
>
>> > Okay, thats a problem.
>
>> >
>
>> > I use features from c++11 standard like "atomic" and "thread"
>
>> >
>
>> > My suggestion:
>
>> >
>
>> > the Pure-C++-Version is is for newer OS / Compiler suits and not
>> > backward
>
>> > compatible (relating compiler / c++ version)
>
>> >
>
>> > Is the way a possible?
>
>>
>
>> Not really, as it requires maintenance of 2 code branches until we can
>
>> completely deprecate the old wxWidgets code. That's why PostgreSQL itself
>
>> is extremely conservative about what they'll support. We're not nearly as
>
>> bad as that, but we do need to carry on supporting RHEL 5 and similarly
>
>> aged OSs for a while.
>
>
>
> I examine whether the use of the boost lib is possible. it should.
>
> I think they has a better backward compatibility.
>
> Please can you review this for me by
> http://www.boost.org/development/tests/release/developer/statechart.html
>
> you know better than i the version of supported compiler / plattforms.

They have a limited list of platforms there, but FYI, RHEL 5 ships
with Boost 1.41.0, so if that works, we're probably OK.


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

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

Am Donnerstag, 19. September 2013, 13:34:47 schrieb Dave Page:

> On Tue, Sep 17, 2013 at 1:59 PM, Linreg <linreg@gmx.net> wrote:

> > Hi Dave,

> >

> >

> >

> > Connection Pooling:

> >

> > okay. I will reimplementing the connection pool.

> >

> > Am Sonntag, 15. September 2013, 15:02:03 schrieb Dave Page:

> >> > > Another problem that I've noticed is that you've unconditionally

> >> > >

> >> > >

> >> > >

> >> > > changed the logging format to be pipe delimited. This is also not

> >> > >

> >> > >

> >> > >

> >> > > acceptable as part of this patch, and should be discussed and

> >> > >

> >> > >

> >> > >

> >> > > implemented separately. At minimum, this would need to be a

> >> > >

> >> > >

> >> > >

> >> > > configurable behaviour change, and by default, I would want it to

> >> > >

> >> > >

> >> > >

> >> > > implement standard (comma delimited) CSV, not a pipe delimited

> >> > >

> >> > >

> >> > >

> >> > > version.

> >> >

> >> > okay. i change this feature to configurable behaviour.

> >> >

> >> >

> >> >

> >> > By default logging will be as before.

> >> >

> >> >

> >> >

> >> > can i add an commandline parameter for this?

> >> >

> >> >

> >> >

> >> > is it than acceptable?

> >>

> >> I think so - but please do that as a separate patch. We don't like to

> >> mixup

> >>

> >> multiple changes in the same patch/commit as it's harder to find issues

> >> or

> >>

> >> review the history in the future.

> >

> > okay. I will make it so.

> >

> >> > Okay, thats a problem.

> >> >

> >> >

> >> >

> >> > I use features from c++11 standard like "atomic" and "thread"

> >> >

> >> >

> >> >

> >> > My suggestion:

> >> >

> >> >

> >> >

> >> > the Pure-C++-Version is is for newer OS / Compiler suits and not

> >> > backward

> >> >

> >> > compatible (relating compiler / c++ version)

> >> >

> >> >

> >> >

> >> > Is the way a possible?

> >>

> >> Not really, as it requires maintenance of 2 code branches until we can

> >>

> >> completely deprecate the old wxWidgets code. That's why PostgreSQL itself

> >>

> >> is extremely conservative about what they'll support. We're not nearly as

> >>

> >> bad as that, but we do need to carry on supporting RHEL 5 and similarly

> >>

> >> aged OSs for a while.

> >

> > I examine whether the use of the boost lib is possible. it should.

> >

> > I think they has a better backward compatibility.

> >

> > Please can you review this for me by

> > http://www.boost.org/development/tests/release/developer/statechart.html

> >

> > you know better than i the version of supported compiler / plattforms.

>

> They have a limited list of platforms there, but FYI, RHEL 5 ships

> with Boost 1.41.0, so if that works, we're probably OK.

Please have an look to this Page

https://access.redhat.com/support/policy/updates/errata/

RHEL5 will be supported until 2017 or with extended support 2020!

you realy want to wait another 4 or 7 years and use old libs and compiler?

Is it not better to make an static linked version or to open a second branch (the number of patches is very low)?

nonetheless i will try to compile it with boost-feature from Boost 1.41.0.

i must remove the atomic-feature. this should be not a problem.

Thomas Steffen

On Thu, Sep 19, 2013 at 3:28 PM, Linreg <linreg@gmx.net> wrote:
>
>> They have a limited list of platforms there, but FYI, RHEL 5 ships
>
>> with Boost 1.41.0, so if that works, we're probably OK.
>
>
>
> Please have an look to this Page
>
> https://access.redhat.com/support/policy/updates/errata/
>
>
>
> RHEL5 will be supported until 2017 or with extended support 2020!
>
> you realy want to wait another 4 or 7 years and use old libs and compiler?
>
> Is it not better to make an static linked version or to open a second branch
> (the number of patches is very low)?

We certainly won't support new versions of pgAgent (or pgAdmin) on
RHEL 5 for that long, but it is still in common use at the moment so
we're not dropping it just yet.

> nonetheless i will try to compile it with boost-feature from Boost 1.41.0.
>
> i must remove the atomic-feature. this should be not a problem.

Thanks.

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

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

Hello Dave,

Excuse my long absence. I had a lot to do.

Here is my patch.

Please note the following:

- The Windows part does not work. I'll make it if you accept this changes.

- The start parameters have a different output format

(all old parameter are the same. NO CHANGES from user is needed! But you

have long-options and no atoi/atol function any more)

- In the CMAKE file I have WX replaced by BOOST.

- I have re-impemented Connection-Pooling

- I have removed CSV-Logoutput-Format

- This Changes i send later.

Thomas Steffen

Neel, can you review this patch please?

Thanks.

On Mon, Oct 14, 2013 at 9:24 AM, Linreg <linreg@gmx.net> wrote:
> Hello Dave,
>
>
>
> Excuse my long absence. I had a lot to do.
>
> Here is my patch.
>
> Please note the following:
>
> - The Windows part does not work. I'll make it if you accept this changes.
>
> - The start parameters have a different output format
>
> (all old parameter are the same. NO CHANGES from user is needed! But you
>
> have long-options and no atoi/atol function any more)
>
> - In the CMAKE file I have WX replaced by BOOST.
>
> - I have re-impemented Connection-Pooling
>
> - I have removed CSV-Logoutput-Format
>
> - This Changes i send later.
>
>
>
> Thomas Steffen
>
>



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

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

Am Donnerstag, 24. Oktober 2013, 11:11:21 schrieb Neel Patel:

> Hi,

>

> Below are the review comments. Compile and tested in Linux.

>

> 1) During compilation we got the following error in "connection.h"

>

> error: pgsql/libpq-fe.h: No such file or directory

>

> To solve the above error we changed "#include <libpg-fe.h>" instead of

> "#include <pgsql/libpq-fe.h>" and it is working fine. Correct me if it is

> some configuration issue.

>

> 2) We are getting the below warning in unix.cpp file which needs to fix.

>

> warning: the use of `tmpnam' is dangerous, better use `mkstemp'.

>

>

> 3) Some of the code is commented and not used so remove the unnecessary

> code.

>

> 4) README file should be modified according to new changes.

>

> 5) In Execute() method in Job.cpp file, we should have to delete "steps"

> from wherever we are returning otherwise it will create the memory leak.

>

>

> Thanks,

> Neel Patel

1.)

it is system dependent. On my SUSE system pibpg-header to be installed under the pgsql directory.

Example: /usr/include/pgsql/libpq-fe.h

2.)

I changed tmpnam to mkdtemp.

I hope thats is okay

3.)

Can you tell me the name of the files. I will delete comments.

4.)

its also my job?

5.)

I have corrected. In the default branch of the switch statement was a return statement(Job.cpp Line: 325). Before that i have a "delete steps," added

Thomas Steffen

On Thu, Oct 24, 2013 at 10:29 AM, Linreg <linreg@gmx.net> wrote:
> Am Donnerstag, 24. Oktober 2013, 11:11:21 schrieb Neel Patel:
>
>> Hi,
>
>>
>
>> Below are the review comments. Compile and tested in Linux.
>
>>
>
>> 1) During compilation we got the following error in "connection.h"
>
>>
>
>> error: pgsql/libpq-fe.h: No such file or directory
>
>>
>
>> To solve the above error we changed "#include <libpg-fe.h>" instead of
>
>> "#include <pgsql/libpq-fe.h>" and it is working fine. Correct me if it is
>
>> some configuration issue.
>
>>
>
>> 2) We are getting the below warning in unix.cpp file which needs to fix.
>
>>
>
>> warning: the use of `tmpnam' is dangerous, better use `mkstemp'.
>
>>
>
>>
>
>> 3) Some of the code is commented and not used so remove the unnecessary
>
>> code.
>
>>
>
>> 4) README file should be modified according to new changes.
>
>>
>
>> 5) In Execute() method in Job.cpp file, we should have to delete "steps"
>
>> from wherever we are returning otherwise it will create the memory leak.
>
>>
>
>>
>
>> Thanks,
>
>> Neel Patel
>
>
>
> 1.)
>
> it is system dependent. On my SUSE system pibpg-header to be installed under
> the pgsql directory.
>
> Example: /usr/include/pgsql/libpq-fe.h

What did the previous code do? That worked. Typically we would not
expect to see a pgsql/ prefix on a header, but would ensure the
compiler is passed the include directory with that directory in it.

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

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

Hi,

I'll change it. (without prefix!)

Thomas

> > > 1.)

> > >

> > > it is system dependent. On my SUSE system pibpg-header to be installed

> >

> > under

> >

> > > the pgsql directory.

> > >

> > > Example: /usr/include/pgsql/libpq-fe.h

> >

> > What did the previous code do? That worked. Typically we would not

> > expect to see a pgsql/ prefix on a header, but would ensure the

> > compiler is passed the include directory with that directory in it.

>

> Previous code is without pgsql/ prefix on a header. So i think we should

> have only "#include <libpg-fe.h>".

>

> > --

> > Dave Page

> > Blog: http://pgsnake.blogspot.com

> > Twitter: @pgsnake

> >

> > EnterpriseDB UK: http://www.enterprisedb.com

> > The Enterprise PostgreSQL Company

Hi,

> > 1.)

> >

> > it is system dependent. On my SUSE system pibpg-header to be

> > installed under the pgsql directory.

> >

> > Example: /usr/include/pgsql/libpq-fe.h

>

> As we discussed we should remove the pgsql/ prefix.

of course, i will correct it.

>

> > 2.)

> >

> > I changed tmpnam to mkdtemp.

> >

> > I hope thats is okay

>

> OK.

>

> > 3.)

> >

> > Can you tell me the name of the files. I will delete comments.

>

> connection.cpp

> job.cpp

> misc.cpp

> pgAgent.h

>

> Also check other files for commented codes.

Okay. I will check it.

> > 4.)

> >

> > its also my job?

>

> I think so because you have only implemented this feature so you are the

> best person to do it. :)

But please do not beat me for my english :)

> > 5.)

> >

> > I have corrected. In the default branch of the switch statement was

> > a return statement(Job.cpp Line: 325). Before that i have

> > a "delete steps," added

>

> That is fine. But what about return statement in Job.cpp:346 ?

Correct.

I hope i have all places of "return" corrected. :)

Hi,

windows port:

okay, I'll try to make it until next week

Am Freitag, 25. Oktober 2013, 12:51:16 schrieb Neel Patel:

> Hi,

>

> One more thing we forgot to add.

>

> Windows part is not working.

>

> @Thomas: Are you going to fix windows part along with this review comments

> and send the updated patch ?

Yes. Okay.

> Thanks,

> Neel Patel

>

> On Fri, Oct 25, 2013 at 12:04 PM, Neel Patel <neel.patel@enterprisedb.com>wrote:

> > Hi,

> >

> > On Thu, Oct 24, 2013 at 2:59 PM, Linreg <linreg@gmx.net> wrote:

> >> **

> >>

> >> Am Donnerstag, 24. Oktober 2013, 11:11:21 schrieb Neel Patel:

> >> > Hi,

> >> >

> >> >

> >> >

> >> > Below are the review comments. Compile and tested in Linux.

> >> >

> >> >

> >> >

> >> > 1) During compilation we got the following error in "connection.h"

> >> >

> >> >

> >> >

> >> > error: pgsql/libpq-fe.h: No such file or directory

> >> >

> >> >

> >> >

> >> > To solve the above error we changed "#include <libpg-fe.h>" instead of

> >> >

> >> > "#include <pgsql/libpq-fe.h>" and it is working fine. Correct me if it

> >>

> >> is

> >>

> >> > some configuration issue.

> >> >

> >> >

> >> >

> >> > 2) We are getting the below warning in unix.cpp file which needs to

> >> > fix.

> >> >

> >> >

> >> >

> >> > warning: the use of `tmpnam' is dangerous, better use `mkstemp'.

> >> >

> >> >

> >> >

> >> >

> >> >

> >> > 3) Some of the code is commented and not used so remove the unnecessary

> >> >

> >> > code.

> >> >

> >> >

> >> >

> >> > 4) README file should be modified according to new changes.

> >> >

> >> >

> >> >

> >> > 5) In Execute() method in Job.cpp file, we should have to delete

> >> > "steps"

> >> >

> >> > from wherever we are returning otherwise it will create the memory

> >> > leak.

> >> >

> >> >

> >> >

> >> >

> >> >

> >> > Thanks,

> >> >

> >> > Neel Patel

> >>

> >> 1.)

> >>

> >> it is system dependent. On my SUSE system pibpg-header to be

> >> installed under the pgsql directory.

> >>

> >> Example: /usr/include/pgsql/libpq-fe.h

> >

> > As we discussed we should remove the pgsql/ prefix.

> >

> >> 2.)

> >>

> >> I changed tmpnam to mkdtemp.

> >>

> >> I hope thats is okay

> >

> > OK.

> >

> >> 3.)

> >>

> >> Can you tell me the name of the files. I will delete comments.

> >

> > connection.cpp

> > job.cpp

> > misc.cpp

> > pgAgent.h

> >

> > Also check other files for commented codes.

> >

> >> 4.)

> >>

> >> its also my job?

> >

> > I think so because you have only implemented this feature so you are the

> > best person to do it. :)

> >

> >> 5.)

> >>

> >> I have corrected. In the default branch of the switch statement was

> >> a return statement(Job.cpp Line: 325). Before that i have

> >> a "delete steps," added

> >

> > That is fine. But what about return statement in Job.cpp:346 ?

> >

> >> Thomas Steffen