tds-fdw / tds_fdw

A PostgreSQL foreign data wrapper to connect to TDS databases (Sybase and Microsoft SQL Server)
Other
377 stars 101 forks source link

Add `--debugging`, `--unattended-debugging` and `--tds_version` parameters #214

Closed jcarnu closed 4 years ago

jcarnu commented 4 years ago

--debugging display PostgreSQL backend PID and waits for key to be pressed allowing gdb --pid=<PID> for debugging purpose (in another session). It also prints the SQL sentences being executed and displays display more precise informations on test crash location using psycopg2.extensions.Diagnostics class

--unattended-debugging does almost the same as debugging, but it doesn't pause, and writes the PostgreSQL logs to console.

--tds_version allows to specify tds version at test time (both in mssql testing and pgsql testing)

This PR will also now print PostgreSQL logs if there are errors.

jenkins-juliogonzalez commented 4 years ago

Test FAILed.

jcarnu commented 4 years ago

Retest this , please

jenkins-juliogonzalez commented 4 years ago

Test PASSed.

jcarnu commented 4 years ago

@juliogonzalez great, I didn't see where the problem was in calling run_tests, now I've seen it :sweat_smile: .

Any comments are more than welcome. I tried to be cleaner in my impl. Maybe run_tests() last param should be show_details than debugging but it's kind of detail.

Next stop tomorrow morning for me.

jenkins-juliogonzalez commented 4 years ago

Test PASSed.

jenkins-juliogonzalez commented 4 years ago

Test PASSed.

jcarnu commented 4 years ago

@juliogonzalez I had time to do the things. Let me know if it's okaye. You've got a collaborator access to my repo.

Sorry not to be more available.

For PGConf.eu maybe next year or maybe could we meet in fosdem in end january/beginning of february ?

jenkins-juliogonzalez commented 4 years ago

Test FAILed.

jenkins-juliogonzalez commented 4 years ago

Test PASSed.

jenkins-juliogonzalez commented 4 years ago

Test FAILed.

jenkins-juliogonzalez commented 4 years ago

Test FAILed.

jenkins-juliogonzalez commented 4 years ago

Test FAILed.

jenkins-juliogonzalez commented 4 years ago

Test FAILed.

juliogonzalez commented 4 years ago

Not my day, it seems. I just need to get this working and print dmesg and postgres log when there are errors or unattended_debugging is enabled.

jenkins-juliogonzalez commented 4 years ago

Test FAILed.

jenkins-juliogonzalez commented 4 years ago

Test PASSed.

juliogonzalez commented 4 years ago

Do not merge the PR please, I am getting the code for the logs ready.

jenkins-juliogonzalez commented 4 years ago

Test PASSed.

juliogonzalez commented 4 years ago

Ok, so this is how it looks like when we use --unattended-debugging:

https://jenkins.juliogonzalez.es/job/tds_fdw-build-free/109/DISTRO=ubuntu16.04,PG_VER=12,label=docker/console https://jenkins.juliogonzalez.es/job/tds_fdw-build-free/109/DISTRO=centos6,PG_VER=12,label=docker/console

As you can see, it prints the queries, and the logs.

For --debugging it will not print the logs (the developer can have a look, and maybe he already has tail tracking them.

Printing logs requires using a PostgreSQL user with SUPERUSER access. If the user is not SUPERUSER there will be an error about it, but that will not fail the script.

In case there are errors, the logs are always printed unless --debugging is being used (useful for the CI, to help having a first look).

jenkins-juliogonzalez commented 4 years ago

Test PASSed.

juliogonzalez commented 4 years ago

@GeoffMontee, @jcarnu, when this PR is approved, let me merge. I will do it with the "squash and merge" option, and will adjust the message. It doesn't make sense having all the commits at master.

juliogonzalez commented 4 years ago

@jcarnu have a look, give it a try, and let me know if we are ready to merge (can't add it you as reviewer).

jcarnu commented 4 years ago

As you know, it will take days for me to have time to check. My training finalization at pgconf is eating all my time.

I will be able to have time maybe on wednesday, not before.

GeoffMontee commented 4 years ago

This looks great to me. I love seeing the new debugging information in the logs. Thanks, @jcarnu and @juliogonzalez!

This gives me an idea for another improvement that we could make to the CI eventually: creating core dumps when PostgreSQL crashes and automatically extracting back traces from them. This would probably make it easier to debug crashes like the one we saw in #213. This would probably require at least the changes listed below. What do you think about that, @juliogonzalez?

Changes Required in ci-build

Changes Required in ci-setup

For example:

sudo yum install postgresql12-debuginfo

For OSes that use systemd, that would probably look like this:

sudo tee /etc/systemd/system/postgresql-12.service.d/limitcore.conf <<EOF
[Service]

LimitCORE=infinity
EOF
sudo systemctl daemon-reload

For other OSes, that would probably look like this:

sudo tee /etc/security/limits.conf.d/postgres_core.conf <<EOF
postgres soft core unlimited
postgres hard core unlimited
EOF

For example:

sudo tee /etc/sysctl.d/postgres_core.conf <<EOF
# Set the path to the core dumps
kernel.core_pattern = /core_dumps

# Add the PID to the end of the file name
kernel.core_uses_pid = 1

# Allow setuid processes to dump core. Is this necessary for Postgres?
fs.suid_dumpable = 2
EOF

For example:

mkdir /core_dumps
chmod 0777 /core_dumps

Changes Required in tds_fdw

2019-10-02 02:28:48.702 UTC [50] LOG:  server process (PID 292) was terminated by signal 11: Segmentation fault

For example:

sysctl kernel.core_pattern

For example:

sudo gdb --batch --eval-command="thread apply all bt full" $(which postmaster) ${core_file_path}
juliogonzalez commented 4 years ago

Yes, it all makes sense. Can you create a new issue with all the content, and I will make some adjustments about what needs to be changed there? :-)

jcarnu commented 4 years ago

I quickly reviewed this patch. It seems to be fine. Thanks for improvement. It can be merged for me.