BIND 10 #3080: New DB backend: Postgres (patch)
BIND 10 Development
do-not-reply at isc.org
Tue Apr 1 13:22:45 UTC 2014
#3080: New DB backend: Postgres (patch)
-------------------------------------+-------------------------------------
Reporter: dclink | Owner: tmark
Type: enhancement | Status:
Priority: medium | reviewing
Component: dhcpdb | Milestone: DHCP-
Keywords: | Kea0.9-alpha
Sensitive: 0 | Resolution:
Sub-Project: DHCP | CVSS Scoring:
Estimated Difficulty: 3 | Defect Severity: N/A
Total Hours: 50 | Feature Depending on Ticket: 2592
| Add Hours to Ticket: 8
| Internal?: 0
-------------------------------------+-------------------------------------
Comment (by tmark):
> ----
> It is not explained anywhere in the bind10-guide that kea requires
special setup for unit tests when using Postgres (compiling with --with-
dhcp-pgsql). Actually the same is the case for MySQL, I think. If this is
explained in the developer's guide there could be a reference to the
developer's guide or at least it should be mentioned that this is in
developer's guide (if not a direct reference).
>
Since the bind10-guide is currently devoid of unit test discussion, I
have not altered this. I think the developer's guide discussion is
sufficient.
> ----
>
> bind10-guide There should be no apostrophe here "postgres=#> GRANT ALL
PRIVILEGES ON DATABASE database-name TO 'user-name;" - before username
>
> ----
>
> The following text in bind10-guide is a little unclear:
>
> {{{
> If instead of seeing keatest=> prompt, your login will be refused with
error code about failed peer or indent authentication, it means that
PostgreSQL is configured to check unix username and reject login attepts
if PostgreSQL names are different. To alter that, PostgreSQL configuration
must be changed. That file is located at
/etc/postgresql/9.1/main/pg_hba.conf on Ubuntu 13.10. Its location may be
different on your system. Please consult your PostgreSQL user manual
before applying those changes as those changes may expose your other
databases that you run on the same system.
> }}}
>
> It would be better to copy-paste exact error messages.
>
> Also, it would be good to explain what exactly has to be changed in the
pg_hba.conf file, e.g. replace !''peer!'' with !''trust!''.
>
> BTW, these are the steps that I used to setup db in postgres:
>
> {{{
> CREATE DATABASE keatest;
> \i /opt/bind10/share/bind10/dhcpdb_create.pgsql
> CREATE USER keatest WITH PASSWORD 'keatest';
> GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public
> \c keatest
> SELECT 'ALTER TABLE '|| tablename ||' OWNER TO keatest;' FROM pg_tables
WHERE SCHEMANAME = 'public'
> edit /etc/postgresql/9.1/main/pg_hba.conf and replace peer with trust
> service postgresql restart
>
> \c keatest
> ALTER TABLE lease6 OWNER TO keatest;
> postgres=# \c keatest
> You are now connected to database "keatest" as user "postgres".
> keatest=# ALTER TABLE lease6 OWNER TO keatest;
> ALTER TABLE
> keatest=# ALTER TABLE lease6_types OWNER TO keatest;
> ALTER TABLE
> keatest=# ALTER TABLE schema_version OWNER TO keatest
>
> }}}
>
> According to our jabber discussion, some of them are redundant, but I
paste it here just for clarity what I did, versus what is in the doc.
>
> Maybe it should be put as a note or warning in the bind10-guide that the
keatest or kea or any other user MUST be an owner of tables!
>
I have reworked this section in guide.
> ----
>
> Postgres backend has invalid logging statement in addLeaseCommon:
> {{{
> LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
>
DHCPSRV_PGSQL_ADD_ADDR4).arg(statements_[stindex].stmt_name);
>
> }}}
>
> It should be logged in the caller function, not here. Also stmt_name is
not the address :)
>
> ----
I deleted this call.
>
> Postgres backend is using MySQL logging messages in several places.
>
> ----
Fixed this.
>
> Not strictly related to this work but... DHCPSRV_ADDRESS6_ALLOC_ERROR -
this error message in alloc_engine is wrong because it talks about IPv6
addresses only. If you fail to allocate the prefix (not a 128 address) it
is quite misleading. And it was misleading when I spotted it for the first
time in the kea log file, when running system tests.
>
> ----
This is out-of-scope for this ticket. I have created #3381 to clarify
IPv6 logging messages to distinguish between allocation types.
>
> I had valgrind failing on Ubuntu 13.10 (gcc 4.7 and 4.8) but I was never
able to reproduce it elsewhere. It worked fine on Debian7. So, maybe my
Ubuntu setup is foo-bared. I attached the log from the valgrind run.
>
> ----
I am unable to reproduce this under Centos6.4/Postgresql 9.3.4, or
Fedora19/Postgresql9.2.6
There are mentions of other people experiencing similiar errors underneath
PQconnectdb:
http://osdir.com/ml/pgsql-admin/2010-06/msg00113.html
http://stackoverflow.com/questions/19484230/checking-memory-with-valgrind
It seems like they tend to boil down to the SSL libraries that are getting
used by PQConnect. Without more investigation on Marcin's failing
environment there
isn't much to be done with it. Should we consider a new ticket?
Other issues, stemming from PGgetresult, should be re-examined as part of
#3382, which calls for a rework of the bind array handling.
>
> There is one thing that I saw initially under valgrind, but I couldn't
reproduce later:
>
> convertToQuery function initializes values three vectors passed by
reference. The ultimate sizes of vectors are equal to size of !''params!''
vector, so given that we never pass empty params vector it should be fine
to do this:
>
> {{{
> convertToQuery(params, out_values, out_lengths, out_formats);
>
> PGresult * r = PQexecPrepared(conn_, statements_[stindex].stmt_name,
> statements_[stindex].stmt_nbparams,
> &out_values[0], &out_lengths[0],
> &out_formats[0], 0);
>
> }}}
>
> Trying to access element with index 0 of out_values and out_formats may
result in segfault if params size is 0. So, in other words, the code could
check for empty vectors returned. But, given that we will remove these
bits of code soon, it is not a MUST.
>
> ----
This is going to go away with #3382.
>
> I found some local variables terminated with underscore (see
addLeaseCommon):
>
> {{{
> vector<const char *> params_;
> vector<int> lengths_;
> vector<int> formats_;
> convertToQuery(params, params_, lengths_, formats_);
> }}}
>
This is going to be reworked with #3382.
>
--
Ticket URL: <https://bind10.isc.org/ticket/3080#comment:17>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list