Created attachment 198420 [details] postgresql11-llvm.patch The LLVM JIT can be built as a separate component port (see patch). But it should be built with the same options as the server — you can see that the bitcode file for the OpenSSL is commented out in the plist because it wasn't built. What do you think?
Why not make this an OPTION? This results in less problems with flag mismatch or mismatching packages. In fact, I've an patch for that...
Created attachment 198443 [details] Alternative: LLVMJIT option on postgresql11-server Let's have a LLVMJIT option on postgresql11-server to enable building the LLVM_JIT-bytecode and support module. Keep this option OFF by default - it nearly doubles package size and pulls in a heavy build dependency (devel/llvm60).
(In reply to Christoph Moench-Tegeder from comment #2) hmm! Can we make flavors? postgresql11-server@jit and @nojit (I think it's important to make the JIT pkg-installable instead of forcing everyone to build from source.)
Yes, my plan is to add them as flavors. Just haven't had the time. Patches are welcome. :)
Why not make the JIT simply an option that is ON by default? If someone is bothered by the huge build dependency, then they can simply turn it off. Long term, I assumes most people will want it.
Is there any progress here? How can i help to get this issue resolved?
Sorry for the delay, I'll look into this in the next few days. Palle
How can we help you Palle? Did you already decide if you want to go with an option (including a flavor or not)? If you decide, we can provide a patch you only need to test. If you are completely out of time, i can test and commit, if you want.
Thanks for the reminder about this one. I could use some help testing it. I'll try adding a flavor, and will post a patch. Palle
How about this one? https://reviews.freebsd.org/differential/diff/53622/ Palle
Mh, i cant download the patch. Can you add me as tester or provide the patch here? At first glance it looks good to me! :)
(In reply to Torsten Zuehlsdorff from comment #11) added you as reviewer
A commit references this bug: Author: girgen Date: Fri Feb 15 11:02:24 UTC 2019 New revision: 492989 URL: https://svnweb.freebsd.org/changeset/ports/492989 Log: The PostgreSQL Global Development Group has released an update to all supported versions of our database system, including 11.2, 10.7, 9.6.12, 9.5.16, and 9.4.21. This release changes the behavior in how PostgreSQL interfaces with `fsync()` and includes fixes for partitioning and over 70 other bugs that were reported over the past three months. Users should plan to apply this update at the next scheduled downtime. FreeBSD port adds OPTIONS knob to support LLVM JIT. [1] Highlight: Change in behavior with fsync() ------------------------------------------ When available in an operating system and enabled in the configuration file (which it is by default), PostgreSQL uses the kernel function `fsync()` to help ensure that data is written to a disk. In some operating systems that provide `fsync()`, when the kernel is unable to write out the data, it returns a failure and flushes the data that was supposed to be written from its data buffers. This flushing operation has an unfortunate side-effect for PostgreSQL: if PostgreSQL tries again to write the data to disk by again calling `fsync()`, `fsync()` will report back that it succeeded, but the data that PostgreSQL believed to be saved to the disk would not actually be written. This presents a possible data corruption scenario. This update modifies how PostgreSQL handles a `fsync()` failure: PostgreSQL will no longer retry calling `fsync()` but instead will panic. In this case, PostgreSQL can then replay the data from the write-ahead log (WAL) to help ensure the data is written. While this may appear to be a suboptimal solution, there are presently few alternatives and, based on reports, the problem case occurs extremely rarely. A new server parameter `data_sync_retry` has been added to manage this behavior. If you are certain that your kernel does not discard dirty data buffers in such scenarios, you can set `data_sync_retry` to `on` to restore the old behavior. Release Notes: https://www.postgresql.org/about/news/1920/ PR: 232490 [1] Changes: head/databases/postgresql10-client/Makefile head/databases/postgresql10-contrib/Makefile head/databases/postgresql10-pgtcl/Makefile head/databases/postgresql10-server/Makefile head/databases/postgresql10-server/distinfo head/databases/postgresql10-server/pkg-plist-client head/databases/postgresql10-server/pkg-plist-server head/databases/postgresql11-server/Makefile head/databases/postgresql11-server/distinfo head/databases/postgresql11-server/pkg-plist-client head/databases/postgresql11-server/pkg-plist-server head/databases/postgresql94-client/Makefile head/databases/postgresql94-contrib/Makefile head/databases/postgresql94-pgtcl/Makefile head/databases/postgresql94-server/Makefile head/databases/postgresql94-server/distinfo head/databases/postgresql94-server/pkg-plist-server head/databases/postgresql95-client/Makefile head/databases/postgresql95-contrib/Makefile head/databases/postgresql95-pgtcl/Makefile head/databases/postgresql95-plperl/Makefile head/databases/postgresql95-plpython/Makefile head/databases/postgresql95-server/Makefile head/databases/postgresql95-server/distinfo head/databases/postgresql95-server/pkg-plist-server head/databases/postgresql96-client/Makefile head/databases/postgresql96-contrib/Makefile head/databases/postgresql96-docs/Makefile head/databases/postgresql96-pgtcl/Makefile head/databases/postgresql96-pltcl/Makefile head/databases/postgresql96-server/Makefile head/databases/postgresql96-server/distinfo head/databases/postgresql96-server/pkg-plist-client head/databases/postgresql96-server/pkg-plist-server
Thanks for the option! :)
Committed the option. I'm also having a dialogue with portsmgr@ about adding a flavour for llvm.
That is great. Do you really should MFH this, since it contains important bugfixes! Greetings, Torsten
(In reply to Torsten Zuehlsdorff from comment #16) Reasonable, will do so in a day or two, if no problems arise.
Shouldn't the LLVM option be using ${LLVM_DEFAULT} from bsd.default-versions.mk rather than hardcoding llvm60?