Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make ZM able to use Postgresql [$250] #355

Open
connortechnology opened this issue Mar 24, 2014 · 36 comments
Open

Make ZM able to use Postgresql [$250] #355

connortechnology opened this issue Mar 24, 2014 · 36 comments
Labels

Comments

@connortechnology
Copy link
Member

I'm creating this as an issue so that people can post bounties to it.

I've already done the php side by converting zm to PDO. However the C needs to be changed and lots of db queries will need to have their syntax adjusted.

There is a $250 open bounty on this issue. Add to the bounty at Bountysource.

@kylejohnson
Copy link
Member

Good call

On March 24, 2014 5:04:10 PM EDT, Isaac Connor notifications@github.com wrote:

I'm creating this as an issue so that people can post bounties to it.

I've already done the php side by converting zm to PDO. However the C
needs to be changed and lots of db queries will need to have their
syntax adjusted.


Reply to this email directly or view it on GitHub:
#355

@jlpoolen
Copy link

Connortechnology: how about staging your work in a branch, I might be willing to address the "db queries" modifications. Perhaps standardizing the SQL overall, if possible, would be in order?

@knight-of-ni
Copy link
Member

@jlpoolen
You mean this branch?
https://github.com/ZoneMinder/ZoneMinder/tree/mysql2PDO

I'm sure he would appreciate your help.

@connortechnology
Copy link
Member Author

The mysql2PDO is a start, since PDO will talk to postgres. However, the SQL queries used do need to be standardized. I will be making a branch for it.

I think the next step is converting the zm_create.sql into Postgres format, so I can at least easily create all the zm tables in my postgres server. There is a lot of abstraction and cleanup to do all over the code.

@jlpoolen
Copy link

knnniggett: yes, thank you.

The title "mysql2PDO" did not clue me into what that project means since I did not know what PDO ( Php Data Objects - http://www.php.net/manual/en/book.pdo.php) was, These are the shortcomings of naming things to familiar acronyms.

PDO -- I'm not familiar with PDO, but on looking at the PDO project, seems to make sense, same kind of abstraction as Perl's DBI (Standard "Database Interface" http://dbi.perl.org/) I'm hoping all of the SQL in ZoneMinder is within the context of PHP.

@connortechnology
Copy link
Member Author

Sadly, not all SQL is in the php. There is lots in perl (not a probem) and lots in C++ code. I'm not sure which C++ library we should use. I did some research and wasn't terribly excited by what I found.

@jlpoolen
Copy link

So, I'm thinking about this approach and would appreciate any thoughts or critques:

  1. export current MYSQL database from a fresh ZoneMinder install to be PostgreSQL compatible (referencing http://en.wikibooks.org/wiki/Converting_MySQL_to_PostgreSQL) or trying an ANSI compatible export to flush out and identify what MySQL idiosyncrasies there may be.

  2. try to build a mirrored schema in PostgreSQL

  3. then go back into ZoneMinder's calls to the database and assess where they are coming from: A) PHP, B) Perl, or C) "c" code and assess what modifications would be in order.

@ariscop
Copy link
Contributor

ariscop commented Dec 8, 2014

I'm looking into this since we're using postgresql at my workplace, few problems i've found:

  1. heavy use of tinyint(n),smallint(n) which don't exist in postgres
  2. enum type needs to be replaced with check constraints or foreign key
  3. one instance of set (Monitors.Triggers) that can probably be removed
  4. No foreign keys anywhere, or constraints of any kind, this is lest mysql->postgres specific and more just confusing

Since i don't have an active installation to work with, would anyone know where i can find some sample data? just for sanity checks while messing with the schema

@jlpoolen
Copy link

jlpoolen commented Dec 8, 2014

I did the schema work already, see:

https://github.com/jlpoolen/ZoneMinder/tree/master/sql

@ariscop
Copy link
Contributor

ariscop commented Dec 8, 2014

Adding sqlite support as well likely won't be too much more work, and would allow zoneminder to run without an external database server

@ariscop
Copy link
Contributor

ariscop commented Dec 10, 2014

libzdb looks suitable. Supports mysql, postgresql, sqlite, oracle, and adds very few dependencies.
libgda supports many more databases but pulls in a glib and a bunch of other libraries, polkit pulls in glib though so it might not be a problem.
Or option c, give zoneminder it's own abstraction layer, though that seems like more work than it's worth with libzdb around now.
I'll start porting to zdb

@ariscop
Copy link
Contributor

ariscop commented Dec 13, 2014

Unfortunately i've had trouble building/running zoneminder, and a number of other projects are in the way. libzdb is suitable and porting is mostly busywork, can afford a small bounty on it if anyones interested.

@kylejohnson kylejohnson changed the title Make ZM able to use Postgresql Dec 17, 2014
@ariscop
Copy link
Contributor

ariscop commented Dec 24, 2014

just found OpenDBX, looks like a better fit, has better db support and a C++ api

@ariscop
Copy link
Contributor

ariscop commented Dec 25, 2014

This could be split into two parallel tasks, one for moving away from mysql client and another for adjusting the schema to be compatible with other db servers.
Zoneminders use of enums is of particular interest, it pulls both string and integer values in a way that i don't think any other db supports.
There's also a lot of duplication of sql in zm_monitor.cpp and the various php skins, could do with a bit of refactoring.

@jlpoolen
Copy link

Did you consider the SQL I created, I endeavored to make it standard.

@ariscop
Copy link
Contributor

ariscop commented Dec 25, 2014

@jlpoolen Yes, there will be a fair amount of work in scripts to upgrade existing installations though.
And schema changes in mysql are not transactional so it has to work flawlessly, or somehow revert it's changes, otherwise it may leave the database in some undefined state..

@jlpoolen
Copy link

@ariscop I'm afraid I'm not understanding the point about "changes in mysql are not transactional." Nevertheless, don't worry about trying to explain it to me, I guess I'm so far away form what I did now I'm out of the loop. I just remember concluding that for purposes of migration, the schema ought to work. Oh well, I hope it's of some help somewhere down the line.

@ariscop
Copy link
Contributor

ariscop commented Dec 28, 2014

It also pulls possible enum values from schema, so that would benefit from being replaced with its own table. Would also make plugable sources easier since the database can ensure you don't remove plugins with existing monitors.

The other issue is a few places where the next auto_incriment value is pulled from the table, the standard way to do this is sequences, which mysql supports, but does not support using them for an auto_incriment style field (default values can't be a function). This may just need more logic, and i don't think it's used in many places so it should be ok.

@kylejohnson kylejohnson changed the title Make ZM able to use Postgresql [$50] Apr 9, 2015
@kylejohnson kylejohnson changed the title Make ZM able to use Postgresql [$150] Jul 15, 2015
@kylejohnson kylejohnson changed the title Make ZM able to use Postgresql [$200] Dec 4, 2015
@JoshuaPK
Copy link

What work is still left on this modification? Also, as far as upgrading the schema: is it a requirement that the scripts do schema upgrades while the system is live, or would it be acceptable for some downtime? Or, could we design the upgrade scripts such that one might do the following:

  1. We create a slightly different (and incompatible) schema to be database agnostic;
  2. Spin up a new ZoneMinder instance that is intended to replace a current ZM instance;
  3. Script would pull the camera and other configuration information from the current instance to the new instance;
  4. "Flip a switch" so that the cameras and monitoring are now running on the new ZM instance;
  5. Script would pull all event and historical data from the old ZM instance and insert into the new ZM instance

I think if we could do a step-by-step version upgrade vs. an in-place upgrade, this modification would be easier.

@connortechnology
Copy link
Member Author

I'm not sure anyone cares about migration. Not at this point at least.

What needs to be done is to change all the code and SQL calls to use some db-agnostic library.

@JoshuaPK
Copy link

Revisiting the abstraction layer again. It looks like OpenDBX hasn't been updated for a while. I've used another package called SOCI, which was updated as recently as two months ago. What do you think about this? https://github.com/SOCI/soci

@connortechnology
Copy link
Member Author

Yeah SOCI looks to be about the right level for our needs. Is also packaged with ubuntu, so that is nice.

@stale
Copy link

stale bot commented Jun 15, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 15, 2018
@stale stale bot closed this as completed Jun 22, 2018
@zdzichu
Copy link

zdzichu commented Jun 23, 2018

Closing this issue while it isn't solved is wrong. Please reopen.

@knight-of-ni
Copy link
Member

Leaving issues open, that no human is interested in touching, is wrong. Thus, until someone steps up and convinces us they will do the work, this issue will remain closed.

@ZoneMinder ZoneMinder locked and limited conversation to collaborators Jun 23, 2018
@stale stale bot removed the stale label Mar 30, 2019
@connortechnology
Copy link
Member Author

I'm reopening because I am going to get around to it soon. Going to make it a milestone for 1.35

@connortechnology
Copy link
Member Author

Bountysource is going to take the money in this bounty on July 1st unless the funds are redirected. Alternatively I can close this out, collect the bounty and ZoneMinder will hold the funds until it is properly closed.

@parvit
Copy link
Contributor

parvit commented Sep 11, 2022

Hi all,
i've been working on this for some time now and i think the direction is clear enough for me to discuss it.
My branch is at https://github.com/parvit/zoneminder/tree/issue-355
Beware it does not even compile all files with queries yet (up to before zm_monitor.cpp at the moment).

The changes are based on using the soci library, they contain at the moment:

  • CMake soci library integration
  • Implementation of the generic db interface and real one using mysql soci backend
  • Implementation of the query executor and queue
  • Porting of part of the zm classes to using the new interface

The steps of the approach are:

  1. first adapt the system to soci with mysql, to check if the whole thing stands
  2. map all queries to prepared statements initialized at the beginning by the selected concrete db backend and reusing it (binding the values in the spot instead of composing the query as text)
  3. after seeing that with mysql works still, implement the postgresql soci backend

Some issues i'm working through:

  • Soci mysql backend does not support nested queries
    • Will rework the 3 queries i've found to use joins instead as i go through the classes
  • Complex structures handled by the queries now need an adapter template class (type_conversion<>) which instructs soci how to handle a particular complex type in terms of the fields returned by a statement
    • For reasons of headers inclusion ordering, i've elected to put the type_conversion of a class in the header the type is declared (at first i tried a single header but it was not feasible without touching a lot more things in the process)
  • I've tried to minimize the syntax and usage of the query class to something manageable, let me know if it's clear enough or any change is necessary

What is missing yet:

  • Porting of the rest of zm classes to using the new interface
  • Check that db class actually executes queries correctly
  • CMake support for the detection and availability of the db modules
  • Configuration support for the selection of the requested backend
  • Implementation of real db interface with postgresql soci backend
  • Check that postgresql soci backend works as intended (in progress)

TLDR;
It's a lot of work ahead still, if possible raising a bit the bounty would be really appreciated.

@GreasyMonkee
Copy link

I am not a coder/developer, but am following this topic with a great deal of interest - am willing and happy to help by adding more to the bounty to help along the work on this.

@jlpoolen
Copy link

I am not a coder/developer, but am following this topic with a great deal of interest - am willing and happy to help by adding more to the bounty to help along the work on this.

Take a look at project moonfire-nvr by Scott Lamb.

@connortechnology
Copy link
Member Author

@jlpoolen This is not an acceptable comment. There are lots of other places to discuss ZoneMinder alternatives. Your comments add nothing relevant to the discussion here.

@GreasyMonkee I'm sure @parvit would appreciate that. This is a huge effort.

@GreasyMonkee
Copy link

Hi @connortechnology @parvit,

Some questions.

  1. Reading through Make ZM able to use Postgresql #3644, I see that you have the YAML for Centos8 there - I am using Fedora 37, I can try to re-write the Centos8 YAML to match for Fedora (finding the appropriate packages, etc)

  2. In Fedora, the Postgresql packaging is a bit of a mess - with what appears to be a "Fedora-specific" package from the Fedora Repositories, and the "official" Postgresql PGDG release packages, with a slightly different naming, i.e. "postgresql" is "postgresql15", as per the "PostgreSQL Global Development Group" naming convention - is it possible to write the YAML in such a fashion that it looks initially for "postgresql" and if not found (repository or packaged disabled, etc), then try "postgresql && $releasever" for the installation?

  3. Further to this, and I am showing my Newb status here - besides the packages listed in the YAML scripts, are there any other packages that are expected to be there? I am totally new to ZoneMinder, so please pardon my ignorance on this.....

@parvit
Copy link
Contributor

parvit commented Jan 7, 2023

@GreasyMonkee Do you have a specific minimum version of fedora in mind you'd like to see? I could create a yaml specific for that.

@GreasyMonkee
Copy link

@parvit I would say Fedora 36, as this is where Postgresql14 was released, however now there is Postgresql15 for Fedora Rawhide (becoming Fedora 38) - I personally will run Fedora 37 on the ZM Server
If I remember correctly there was some significant changes before Fedora 36, so I would not try to go back further than that.

@parvit
Copy link
Contributor

parvit commented Jan 7, 2023

@GreasyMonkee Added the flow for fedora 36, works locally in docker but won't work on the builders until the master is fixed too. Fixed

@connortechnology The tests on freebsd should be fixed by bumping the image version to freebsd 13.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
9 participants