----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51284/#review146363 -----------------------------------------------------------
Out of curiousity, did you happen to compare the content of `Ambari-DDL-Postgres-EMBEDDED-CREATE.sql` with `Ambari-DDL-Postgres-CREATE.sql` to ensure they were identical? ambari-server/src/main/python/ambari_server/dbConfiguration_linux.py (line 116) <https://reviews.apache.org/r/51284/#comment212759> Upon re-running the setup, wouldn't this always print the message: Database admin user (postgres): Even if they've previously configured this for another user? ambari-server/src/main/python/ambari_server/dbConfiguration_linux.py (lines 664 - 674) <https://reviews.apache.org/r/51284/#comment212762> So here, you're letting the framework run `Ambari-DDL-Postgres-EMBEDDED-CREATE.sql` like it normally would to bootstrap creation of the schema and ambari user. And then, assuming that worked, you're then running the DDL stuff as the new user (and not postgres). If this DDL stuff fails, the error message should indicate that instead of the generic "Connection Timed Out", which could be wrong. ambari-server/src/main/python/ambari_server/dbConfiguration_linux.py (lines 765 - 773) <https://reviews.apache.org/r/51284/#comment212764> This is going to be run on every upgrade, right? Can we change this message to be a little clearer (maybe removed the term "tweaked")? Also - should we have a very simple SQL we can execute to test to ensure we can run SQL statements as `postgres`; if that fails, don't even try the more complex stuff? New text: ``` Ambari is unable to change ownership of the database tables in {database} to {user}. This may be because the administrator user ({admin_user}) does not have permission to make the changes. To make sure that all tables are owned by {user}, you can execute the following SQL: "SELECT tablename FROM pg_tables WHERE schemaname = '{schema}';", "SELECT sequence_name FROM information_schema.sequences WHERE sequence_schema = '{schema}';", "SELECT table_name FROM information_schema.views WHERE table_schema = '{schema}'; ``` ambari-server/src/main/python/ambari_server/dbConfiguration_linux.py (line 783) <https://reviews.apache.org/r/51284/#comment212766> Documentation. ambari-server/src/main/python/ambari_server/dbConfiguration_linux.py (line 793) <https://reviews.apache.org/r/51284/#comment212763> Documentation. ambari-server/src/main/python/ambari_server/dbConfiguration_linux.py (lines 828 - 830) <https://reviews.apache.org/r/51284/#comment212768> Why not just use the return code here to see if the command succeeded? Do we get a 0 return code on failed SQL statements? ambari-server/src/main/python/ambari_server/dbConfiguration_linux.py (line 835) <https://reviews.apache.org/r/51284/#comment212765> Documentation. ambari-server/src/main/python/ambari_server/serverConfiguration.py (lines 144 - 145) <https://reviews.apache.org/r/51284/#comment212761> Let's add a comment to describe exactly what this is. Something like: # The user which will bootstrap embedded postgres database setup by creating the default schema and ambari user. ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql (line 28) <https://reviews.apache.org/r/51284/#comment212767> Can you verify that this will ensure that commands run as the Ambari DB user are automatically scoped to the `ambari` schema? ambari-server/src/test/python/unitTests.py (lines 207 - 208) <https://reviews.apache.org/r/51284/#comment212760> This doesn't seem like a good test anymore. - Jonathan Hurley On Aug. 22, 2016, 5:39 a.m., Andrew Onischuk wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51284/ > ----------------------------------------------------------- > > (Updated Aug. 22, 2016, 5:39 a.m.) > > > Review request for Ambari, Jonathan Hurley and Vitalyi Brodetskyi. > > > Bugs: AMBARI-18226 > https://issues.apache.org/jira/browse/AMBARI-18226 > > > Repository: ambari > > > Description > ------- > > There are currently two SQL files which are being used to initialized Postgres > databases. > > * > [Ambari-DDL-Postgres-CREATE.sql](https://github.com/apache/ambari/blob/trunk/ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql) > * > [Ambari-DDL-Postgres-EMBEDDED-CREATE.sql](https://github.com/apache/ambari/blob/trunk/ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql) > > There should be no need to duplicate all of the SQL DDL/DML between these two > files. It's error-prone and cumbersome to maintain. > > Instead, the problem seems to be that the embedded SQL doesn't actually switch > users after it bootstraps everything: > > > > > CREATE DATABASE :dbname; > \connect :dbname; > > ALTER ROLE :username LOGIN ENCRYPTED PASSWORD :password; > CREATE ROLE :username LOGIN ENCRYPTED PASSWORD :password; > > GRANT ALL PRIVILEGES ON DATABASE :dbname TO :username; > > CREATE SCHEMA ambari AUTHORIZATION :username; > ALTER SCHEMA ambari OWNER TO :username; > ALTER ROLE :username SET search_path TO 'ambari'; > > ------create tables and grant privileges to db user--------- > CREATE TABLE ambari.stack( > stack_id BIGINT NOT NULL, > ... > GRANT ALL PRIVILEGES ON TABLE ambari.stack TO :username; > > > This causes several problems: > > * Because tables are being creating from the `postgres` user instead of > `:username`, they need to be altered to have privileges granted. > * Because tables are being creating from the `postgres` user instead of > `:username`, the default `search_path` is wrong and needs to be prefixed to > all calls. > > Instead, the embedded SQL should leverage the remote SQL for all of the table > creation and data seeding. The embedded SQL should only be responsible for > bootstrapping the database, schema, and user. > > > Diffs > ----- > > ambari-server/src/main/python/ambari_server/dbConfiguration_linux.py > c595e41 > ambari-server/src/main/python/ambari_server/serverConfiguration.py 65bf55c > ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql > 43bdef9 > ambari-server/src/main/resources/scripts/change_owner.sh 6f0ac3d > ambari-server/src/test/python/TestAmbariServer.py a45a4bd > ambari-server/src/test/python/unitTests.py 037b6a5 > > Diff: https://reviews.apache.org/r/51284/diff/ > > > Testing > ------- > > mvn clean test > > > Thanks, > > Andrew Onischuk > >
