Bug 277262 - databases/foundationdb: Large-scale port update
Summary: databases/foundationdb: Large-scale port update
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-ports-bugs (Nobody)
URL: https://apple.github.io/foundationdb/...
Keywords:
Depends on:
Blocks:
 
Reported: 2024-02-23 14:53 UTC by Dmitry Wagin
Modified: 2024-05-23 22:15 UTC (History)
1 user (show)

See Also:
bugzilla: maintainer-feedback? (dch)


Attachments
foundationdb.diff (41.70 KB, patch)
2024-02-23 14:53 UTC, Dmitry Wagin
dmitry.wagin: maintainer-approval+
Details | Diff
foundationdb.diff (54.91 KB, patch)
2024-02-25 17:34 UTC, Dmitry Wagin
no flags Details | Diff
single fdb version, appease linters, split client & server, update erlfdb (47.12 KB, patch)
2024-03-16 14:41 UTC, Dave Cottlehuber
no flags Details | Diff
foundationdb.diff (55.65 KB, patch)
2024-04-01 04:25 UTC, Dmitry Wagin
no flags Details | Diff
foundationdb.diff (55.49 KB, patch)
2024-04-14 06:33 UTC, Dmitry Wagin
dmitry.wagin: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Wagin 2024-02-23 14:53:13 UTC
Created attachment 248698 [details]
foundationdb.diff

* Pick up maintainership, as requested by Dave Cottlehuber in bug #266207

* Separate port for client library

* Separate port for 7.1.X and 7.3.X

* Update 7.1.X to 7.1.57
Changelog: https://github.com/apple/foundationdb/blob/7.1.57/documentation/sphinx/source/release-notes/release-notes-710.rst

* Add 7.3.33
Changelog: https://github.com/apple/foundationdb/blob/7.3.33/documentation/sphinx/source/release-notes/release-notes-730.rst
Comment 1 Dmitry Wagin 2024-02-25 17:34:12 UTC
Created attachment 248742 [details]
foundationdb.diff

some cleanup
Comment 2 Dave Cottlehuber freebsd_committer freebsd_triage 2024-02-28 16:18:33 UTC
happy to commit this, 2 questions first:

- do we really want/need 3 fdb versions in tree?

I'm sure you have a reason, but IIRC the protocol should support clients
built against a lower version to use a newer fdb server, and its a
significant build impact on the pkg servers for a comparatively small
user base.

- do you have a git branch somewhere I can fetch it from?

poudriere is building atm ...
Comment 3 Dmitry Wagin 2024-02-28 21:21:23 UTC
(In reply to Dave Cottlehuber from comment #2)
> do we really want/need 3 fdb versions in tree?

Not 3, only 2 7.1.X and 7.3.X

> do you have a git branch somewhere I can fetch it from?

No, only attached patch, apply it via "git apply".
Comment 4 Dave Cottlehuber freebsd_committer freebsd_triage 2024-02-29 08:06:52 UTC
none of these built in poudriere, 

https://pkg.skunkwerks.at/poudriere/build.html?mastername=current_x64-default&build=2024-02-28_16h19m03s


-- ADDING SIMULATOR TEST 194 status/separate_not_enough_servers
-- ADDING SIMULATOR TEST 195 status/single_process_too_many_config_params
CMake Error at /usr/local/share/cmake/Modules/ExternalProject.cmake:2910 (message):
  error: could not find git for clone of googlebenchmark
Call Stack (most recent call first):
  /usr/local/share/cmake/Modules/ExternalProject.cmake:4418 (_ep_add_download_command)
  CMakeLists.txt:5 (ExternalProject_Add)

and CMakeFiles/fdbserver.dir/DDTeamCollection.actor.g.cpp.o /wrkdirs/usr/ports/databases/foundationdb73-client/work/.build/fdbserver/DDTeamCollection.actor.g.cpp
1.	/wrkdirs/usr/ports/databases/foundationdb73-client/work/foundationdb-7.3.33/fdbserver/DDTeamCollection.actor.cpp:6437:3 <Spelling=/wrkdirs/usr/ports/databases/foundationdb73-client/work/foundationdb-7.3.33/flow/include/flow/Error.h:128:51>: current parser token ')'
2.	/wrkdirs/usr/ports/databases/foundationdb73-client/work/foundationdb-7.3.33/fdbserver/DDTeamCollection.actor.cpp:5864:1: parsing struct/union/class body 'DDTeamCollectionUnitTest'
3.	/wrkdirs/usr/ports/databases/foundationdb73-client/work/.build/fdbserver/DDTeamCollection.actor.g.cpp:28669:2: parsing function body 'DDTeamCollectionUnitTest::GetTeam_ServerUtilizationNearCutoffActorState::a_body1cont1'
4.	/wrkdirs/usr/ports/databases/foundationdb73-client/work/.build/fdbserver/DDTeamCollection.actor.g.cpp:28669:2: in compound statement ('{}')
5.	/wrkdirs/usr/ports/databases/foundationdb73-client/work/foundationdb-7.3.33/fdbserver/DDTeamCollection.actor.cpp:6437:3 <Spelling=/wrkdirs/usr/ports/databases/foundationdb73-client/work/foundationdb-7.3.33/flow/include/flow/Error.h:127:5>: in compound statement ('{}')

I'll try on 14.0-amd64 today and let you know how that goes.
Comment 5 Dave Cottlehuber freebsd_committer freebsd_triage 2024-02-29 08:25:11 UTC
poudriere 14.o-amd64 reports:

client:configure - same issue as before
[00:20:18] Built ports: devel/libfmt databases/foundationdb73-client databases/foundationdb73-server
[00:20:18] Failed ports: databases/foundationdb71-client:configure
[00:20:18] Skipped ports: databases/foundationdb71-server
[00:20:16] [01] [00:09:38] Finished databases/foundationdb73-server | foundationdb73-server-7.3.33: Success

https://pkg.skunkwerks.at/poudriere/build.html?mastername=14_0_x64-default&build=2024-02-29_08h03m49s
Comment 6 Dmitry Wagin 2024-03-04 19:06:41 UTC
(In reply to Dave Cottlehuber from comment #5)

I am building it on FreeBSD:14:amd64, maybe there's something wrong with the patch.
Comment 7 Dave Cottlehuber freebsd_committer freebsd_triage 2024-03-05 15:10:00 UTC
are you doing it in poudriere? if not, it will pick up stray state from your
filesystem, and also will have network access, which is what's breaking those.

If it can't build in poudriere on a 15.0-CURRENT kernel, and a 14.0-RELEASE
jail, then this will also fail in official package builders, which are the
same setup.

Do you have a git repo with a branch and your commits in? I can see if that
helps.

Also please clarify why you're splitting the ports, FDB clients specifically
import a .h file with DFDB_API_VERSION to allow a client to connect to a
newer server version than what the client knows of:

        "$CFLAGS -I/usr/local/include -Ic_src/ -g -Wall -Werror -DFDB_API_VERSION=" ++ MaxAPIVersion ...

(from erlfdb port as example).

I'd suggest we get 1 update and working version in place, and then split
into separate versions as required. Up to you!
Comment 8 Dmitry Wagin 2024-03-06 18:36:44 UTC
(In reply to Dave Cottlehuber from comment #7)
> are you doing it in poudriere?

Yes, of course.

> Do you have a git repo with a branch and your commits in? I can see if that
helps.

https://gitlab.com/dwagin/freebsd-ports/-/commit/08794524940435ff75779530842f4e2cf83cd86f

> Also please clarify why you're splitting the ports

In order for packaging to be the same as in other systems.

> I'd suggest we get 1 update and working version in place, and then split
into separate versions as required. Up to you!

I'm currently using the 7.3.X branch, 7.1.X has been left for those who can use it.
Comment 9 Dave Cottlehuber freebsd_committer freebsd_triage 2024-03-16 11:21:00 UTC
thanks for the git link, much easier!

Dmitry you are right, 7.3 won't build on FreeBSD current (at least in poudriere)
because llvm version is newer and stuff breaks in subtle ways. This doesn't
need to block the port update ofc, but we'll need a `BROKEN_FreeBSD_15=      Incompatible with LLVM17 in 15-CURRENT` while we find a fix.

For the port itself, foundationdb already provides a "backwards-compatible"
header file for clients to use and compile against. This means there's no
need to have 2 separate port versions in the ports tree, and clients are
able to negotiate the required FDB protocol directly.

> FDB clients specifically import a .h file with DFDB_API_VERSION to allow
> a client to connect to a newer server version than what the client knows:
> "$CFLAGS -I/usr/local/include -Ic_src/ -g -Wall -Werror -DFDB_API_VERSION=" ++ MaxAPIVersion ...

Can you add a new update that does the nice split of server & client, but
doesn't make a separate 7.1 & 7.3 port? IMO this should be ready to go!
Comment 10 Dave Cottlehuber freebsd_committer freebsd_triage 2024-03-16 14:41:18 UTC
Created attachment 249218 [details]
single fdb version, appease linters, split client & server, update erlfdb

how does this look? I just renamed your 7.33.3, tidied up portclippy & portfmt,
and included my LLVM version constraint for 15 builds. Looking good!

poudriere tests for 14.0 amd64 and 15-CURRENT amd64 are running atm.
Comment 11 Dmitry Wagin 2024-03-16 21:12:32 UTC
(In reply to Dave Cottlehuber from comment #10)

https://apple.github.io/foundationdb/api-general.html#api-versions

By default, client and server binaries installed for a given cluster must have the same version. However, there is also a multi-version feature that allows a client to connect to a cluster whose server processes have a different version. This feature works by loading a separate client library that is version-compatible with the cluster, which then proxies all API calls.

https://forums.foundationdb.org/t/client-library-backwards-compatibility-issues/919

For a given FDB version major.minor.patch, only .patch-level releases are protocol compatible. This means any major or minor changes to the server imply a new protocol across the cluster. The fdb_c library uses this same protocol to talk to the cluster, so it impacts all clients, as well.



Please provide a link to the "backwards compatible" client library documentation.

In my experience, you need the same version of the server and fdb_c library.

I don’t have a test stand right now to confirm this.
Comment 12 Dave Cottlehuber freebsd_committer freebsd_triage 2024-03-17 23:20:11 UTC
TLDR you are right :-)

From the docs https://apple.github.io/foundationdb/api-general.html
~~~
In general, a given client will support both its corresponding API version and earlier API versions. A client selects the API version it will use by explicitly specifying the version number upon initialization. The purpose of this design is to allow the server, client libraries, or bindings to be upgraded without having to modify application code. You can therefore upgrade to more recent packages (and so receive their various improvements) without having to change application code.
~~~

I've mis-interpreted this as being able to upgrade the server & client
without downtime. 

I had a longer read of docs, and tried this in practice. I was confused
by the difference between client requested API version, and the wire
binary/network protocol, these things are not the same.

This means that any upgrade requires:

- installing both libfdb_c.so versions on *all* clients and restarting them before
  upgrading servers
- bouncing all servers within a very short timeframe once client updates are done

IMO the ports tree doesn't need to cater for this scenario, users will need
to plan their upgrades themselves and manage their server versions very very
closely.

The alternative is that the ports tree accumulates multiple FDB versions,
for every single MAJOR.MINOR release, which is a tremendous waste of 
build time and space in the cluster.

Keeping fdb versions to "latest" version in main repo seems a reasonable
compromise.

What do you think?
Comment 13 Dave Cottlehuber freebsd_committer freebsd_triage 2024-03-23 10:05:35 UTC
the alternative is to have each version in ports install a different-versioned
libfdb_c.so such that the clients can be installed side-by-side. Is that an
option?
Comment 14 Dmitry Wagin 2024-03-23 18:21:20 UTC
(In reply to Dave Cottlehuber from comment #13)

> included my LLVM version constraint for 15 builds.

Hello Dave,

please tell me more about this.

I also added portclippy changes to the gitlab branch.

https://gitlab.com/dwagin/freebsd-ports/-/commits/dwagin-foundationdb?ref_type=heads
Comment 15 Dave Cottlehuber freebsd_committer freebsd_triage 2024-03-24 15:19:39 UTC
We discovered that FDB won't build on 15.0-CURRENT because the newer LLVM
produces errors. While that bug is addressed, we can work around it in
ports by requiring LLVM=<17:

USES=           cmake compiler:c++20-lang llvm:max=16 mono:build python:build \
                shebangfix ssl

On 13 & 14, builtin llvm is always max 16, so this only alters behaviour
on CURRENT.
Comment 16 Dmitry Wagin 2024-04-01 04:25:02 UTC
Created attachment 249612 [details]
foundationdb.diff

* max llvm 16
* 7.1.59 and 7.3.35
* erlfdb
* portclippy
Comment 17 Dmitry Wagin 2024-04-01 04:31:03 UTC
(In reply to Dave Cottlehuber from comment #13)

IMHO, the ports tree should contain multiple versions of FDB for each individual MAJOR.MINOR release.

All databases in the ports tree do this.
Comment 18 commit-hook freebsd_committer freebsd_triage 2024-04-11 10:06:11 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=c7911fcef4f31ad76b2c6369ca03d5a100b236d2

commit c7911fcef4f31ad76b2c6369ca03d5a100b236d2
Author:     Dave Cottlehuber <dch@FreeBSD.org>
AuthorDate: 2024-04-11 10:04:28 +0000
Commit:     Dave Cottlehuber <dch@FreeBSD.org>
CommitDate: 2024-04-11 10:04:30 +0000

    database/foundationdb: update maintainer

    PR:             277262

 databases/foundationdb/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 19 Dmitry Wagin 2024-04-14 06:33:16 UTC
Created attachment 249961 [details]
foundationdb.diff

* 7.1.59 and 7.3.37
* rebase
Comment 20 Dave Cottlehuber freebsd_committer freebsd_triage 2024-05-22 21:09:45 UTC
hi Dmitry,

do you have a version of this patch that can be applied cleanly against the ports tree?
We should get this committed soon!
Comment 21 Dmitry Wagin 2024-05-23 12:44:42 UTC
(In reply to Dave Cottlehuber from comment #20)

Hi Dave,

fresh rebase is here:

https://gitlab.com/dwagin/freebsd-ports/-/tree/dwagin-foundationdb
Comment 22 commit-hook freebsd_committer freebsd_triage 2024-05-23 22:15:01 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=3dc4928b6c995e22f199f7b9a1bdb5247d8d7a10

commit 3dc4928b6c995e22f199f7b9a1bdb5247d8d7a10
Author:     Dmitry Wagin <dmitry.wagin@ya.ru>
AuthorDate: 2024-05-23 22:14:48 +0000
Commit:     Dave Cottlehuber <dch@FreeBSD.org>
CommitDate: 2024-05-23 22:14:48 +0000

    databases/foundationdb*: split, update to 7.1.59 & 7.3.41

    Update FoundationDB to both main supported versions, and also split
    between server and client builds for convenience.

    PR:             277262
    Reviewed by:    dch
    Sponsored by:   SkunkWerks, GmbH

 MOVED                                              |   2 +
 UIDs                                               |   2 +-
 databases/Makefile                                 |   5 +-
 databases/erlfdb/Makefile                          |   2 +-
 databases/foundationdb/Makefile (gone)             |  98 -------------------
 databases/foundationdb/distinfo (gone)             |   3 -
 .../files/patch-cmake_CompileBoost.cmake (gone)    |  11 ---
 .../files/patch-cmake_GetMsgpack.cmake (gone)      |  24 -----
 .../files/patch-fdbmonitor_CMakeLists.txt (gone)   |  11 ---
 databases/foundationdb/pkg-descr (gone)            |  14 ---
 databases/foundationdb/pkg-plist (gone)            |  28 ------
 databases/foundationdb71-client/Makefile (new)     |  11 +++
 databases/foundationdb71-server/Makefile (new)     | 103 ++++++++++++++++++++
 databases/foundationdb71-server/distinfo (new)     |   3 +
 .../files/foundationdb.conf.in                     |   6 +-
 .../files/foundationdb.in                          |   2 +-
 .../files/patch-CMakeLists.txt                     |  13 +--
 .../files/patch-bindings_CMakeLists.txt (new)      |  10 ++
 .../files/patch-bindings_c_CMakeLists.txt          |   6 +-
 .../files/patch-cmake_CompileBoost.cmake (new)     |  20 ++++
 .../files/patch-cmake_FDBComponents.cmake          |   8 +-
 .../files/patch-cmake_GetMsgpack.cmake (new)       |  17 ++++
 .../files/patch-fdbmonitor_CMakeLists.txt (new)    |   8 ++
 .../files/patch-fdbmonitor_fdbmonitor.cpp          |   0
 .../files/patch-fdbserver_FDBExecHelper.actor.cpp  |   0
 .../files/patch-fdbserver_fdbserver.actor.cpp      |   4 +-
 databases/foundationdb71-server/pkg-descr (new)    |   8 ++
 .../foundationdb71-server/pkg-plist-client (new)   |  13 +++
 .../foundationdb71-server/pkg-plist-server (new)   |   6 ++
 databases/foundationdb73-client/Makefile (new)     |  11 +++
 databases/foundationdb73-server/Makefile (new)     | 108 +++++++++++++++++++++
 databases/foundationdb73-server/distinfo (new)     |   3 +
 .../files/foundationdb.conf.in (new)               |  47 +++++++++
 .../files/foundationdb.in (new)                    |  42 ++++++++
 .../files/patch-CMakeLists.txt (new)               |  15 +++
 .../files/patch-bindings_CMakeLists.txt (new)      |  11 +++
 .../files/patch-bindings_c_CMakeLists.txt (new)    |  64 ++++++++++++
 ...dings_c_test_apitester_TesterWorkload.cpp (new) |  11 +++
 .../patch-bindings_c_test_mako_mako.cpp (new)      |  11 +++
 .../files/patch-cmake_CompileBoost.cmake (new)     |  20 ++++
 .../files/patch-cmake_FDBComponents.cmake (new)    |  32 ++++++
 .../files/patch-cmake_GetMsgpack.cmake (new)       |  17 ++++
 .../files/patch-fdbcli_CMakeLists.txt (new)        |  11 +++
 ...patch-fdbclient_SpecialKeySpace.actor.cpp (new) |  11 +++
 .../files/patch-fdbmonitor_CMakeLists.txt (new)    |   8 ++
 .../files/patch-fdbmonitor_fdbmonitor.cpp (new)    |  44 +++++++++
 .../patch-fdbserver_BlobWorker.actor.cpp (new)     |  33 +++++++
 .../patch-fdbserver_storageserver.actor.cpp (new)  |  20 ++++
 .../files/patch-flow_MkCertCli.cpp (new)           |  11 +++
 .../files/patch-flow_Platform.actor.cpp (new)      |  21 ++++
 databases/foundationdb73-server/pkg-descr (new)    |   8 ++
 .../foundationdb73-server/pkg-plist-client (new)   |  16 +++
 .../foundationdb73-server/pkg-plist-server (new)   |   6 ++
 53 files changed, 805 insertions(+), 214 deletions(-)
Comment 23 Dave Cottlehuber freebsd_committer freebsd_triage 2024-05-23 22:15:34 UTC
\o/