Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configure FDB_API_VERSION dynamically #37

Merged
merged 5 commits into from
Nov 9, 2021
Merged

Conversation

kocolosk
Copy link
Member

@kocolosk kocolosk commented Nov 8, 2021

This patch set dynamically detects the maximum API version supported by the installed FDB client library and builds the NIF wrapper against that version automatically.

Older API versions can still be selected at runtime as usual through the application environment; this just ensures we have the largest possible range at our disposal, without restricting the versions of the client library we can support with a single branch of this codebase.

Closes #34
Closes #38

VsnInfo = os:cmd("fdbcli --version"),
{match, [ProtocolStr]} = re:run(VsnInfo, "protocol ([a-f0-9]*)", [{capture, [1], list}]),
ProtocolVsn = list_to_integer(ProtocolStr, 16),
APIVersionBytes = (ProtocolVsn band 16#0000000FFF00000) bsr 20,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's quite neat!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah, I thought it seemed quite hacky, but looking at the way it's defined in FDB I suspect this ends up being pretty reliable

Copy link
Contributor

@nickva nickva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A very nice improvement

There is a note about 610 support but otherwise if the build passes +1

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@kocolosk kocolosk changed the title Configure fdb version Configure FDB_API_VERSION dynamically Nov 8, 2021
@kocolosk
Copy link
Member Author

kocolosk commented Nov 8, 2021

While we are technically green on 6.3 here, it seems there are a handful of changes in API version 630 that we ought to be accounting for:

https://apple.github.io/foundationdb/api-version-upgrade-guide.html#api-version-630

Specifically:

  • The get_addresses_for_key function now returns strings that include the port in the address. Prior to API version 630, this required using the INCLUDE_PORT_IN_ADDRESS option, which has now been deprecated.
  • The ENABLE_SLOW_TASK_PROFILING network option has been replaced by ENABLE_RUN_LOOP_PROFILING and is now deprecated.
  • The FDBKeyValue struct’s key and value members have changed type from void* to uint8_t*.

@kocolosk
Copy link
Member Author

kocolosk commented Nov 9, 2021

Will squash the CI commits into one to clean up the history, but this is stacked on top of #33 so I think I may wait until that one is merged to proceed further.

This is the new name for ENABLE_SLOW_TASK_PROFILING.
Base automatically changed from erlfmt to main November 9, 2021 12:33
@kocolosk kocolosk merged commit aaeda1d into main Nov 9, 2021
@kocolosk kocolosk deleted the configure-fdb-version branch November 9, 2021 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants