MisterRaindrop opened a new pull request, #2044:
URL: https://github.com/apache/cloudberry/pull/2044
### What does this PR do?
`contrib/datalake_fdw` reads and writes Parquet (#1951), but only on a local
file system. This adds the storage layer underneath: an `s3` backend over the
AWS SDK for C++, and a published contract so a backend this extension does
not
ship can be added from outside it.
Closes #2009.
Decisions worth a look:
- **A backend answers one question**: given a location and its options, which
`arrow::fs::FileSystem` reads and writes it. Opening files, listing,
classifying errors, accounting for memory and translating a URI into a
native
path all stay on this extension's side of the boundary, so a backend is a
mount function and nothing else.
- **`s3` is our own `arrow::fs::FileSystem` over the AWS SDK**, not Arrow's.
No RPM of Arrow is built with S3 support -- EPEL's and the Arrow project's
own both set `use_s3 0` -- so depending on it would mean asking every user
to
build Arrow. The SDK is a `BUILD_ONLY="s3;sts"` static build that takes
well
under a minute, links statically, and adds libcurl, OpenSSL and zlib to
`NEEDED` and nothing else.
- **Timeouts and retries are set by this module**, not inherited: connect 5
s,
request 300 s, three retries, so a black-holed endpoint answers in about 24
seconds rather than hanging a session, on every Arrow version.
- **A writer only ever removes what it created.** The local backend opens
with
`O_EXCL` and unlinks that path alone; the S3 one keys cleanup on whether an
upload is still live rather than on whether the stream is closed, so a
`Close()` that fails at the last part still aborts the upload instead of
leaving parts to be billed for.
- **Credentials are scrubbed at one place.** Every value under a key
containing
`secret`, `token` or `password` is remembered per process, and
`dl_error_set`
-- the single exit every `DlErrCode` error goes through -- removes it from
the message. An unexpected C++ exception and a third-party backend's own
status are covered by construction rather than by remembering to call
something. A `base_path` that carries a password in its userinfo is
redacted
the same way, in the message as well as the detail.
- **Which schemes a volume may name comes from the registry**, not a list in
the parser: a backend that registered `mine` can be written into a
`base_path` without this extension being changed.
- **The plug-in contract is versioned three ways**: an ABI version, an Arrow
version, and a fingerprint over the compiler major and
`_GLIBCXX_USE_CXX11_ABI`.
Registration happens during preload and in any order -- the registering
side
pulls in `datalake_fdw` through `load_external_function`, inside a `PG_TRY`
so an `ereport` cannot unwind through the plug-in's C++ frames.
- **Every Arrow allocation still goes through the tracked pool**, including a
backend's: `mount` is handed a host struct carrying it, and the five
installed headers give a backend no way to reach Arrow's default pool by
accident.
### Type of Change
- [ ] Bug fix (non-breaking change)
- [x] New feature (non-breaking change)
- [ ] Breaking change (fix or feature with breaking changes)
- [ ] Documentation update
### Test Plan
One body of storage behaviour runs against every backend rather than once per
backend: `storage_conformance` parameterises a shared SQL file over a URI
prefix and a volume, and runs it for `file://`, for `s3://` and for the test
backend.
- [x] Integration tests added -- three new categories. `storage_local` covers
the facade and the registry: four kinds of bad registration, a
duplicate
scheme, path escapes, a name already in use, a missing file.
`storage_s3` covers the backend against a real service: round trip,
9 MB through multipart, `ListObjectsV2` paging, `HeadObject`,
`DeleteObject`, deleting twice, a missing key, a missing bucket, a
wrong
secret (and that the message does not contain it), path style with an
endpoint override, and a black-holed endpoint inside 30 s.
`storage_conformance` adds the type round trip, field-id projection, a
row-group range, listing, a rejected overwrite, a write that fails
halfway leaving nothing behind, a prefix nothing was written under and
one whose objects were just deleted (both not-found, which is the
point:
object storage and a filesystem disagree and the facade decides), and
volume resolution: no `USAGE`, a `PUBLIC` mapping, a URI outside its
volume, a volume that does not exist, and a rejected `base_path` that
must not echo its own password.
- [x] Passed `make installcheck` -- every category, on a three-segment
cluster
with the module preloaded, against MinIO and against SeaweedFS, on
Arrow 9.0.0 and Arrow 17.0.0.
- [ ] Unit tests added/updated
- [ ] Passed `make -C src/test installcheck-cbdb-parallel` (not run)
Beyond the suite:
- **The plug-in contract was used from outside.** A `.cc` file that includes
only the five installed headers and PostgreSQL's server headers compiles
to a
`.so` that registers a backend; put *before* `datalake_fdw` in
`shared_preload_libraries` the cluster starts and the scheme works, and
built with a stale `abi_fingerprint` the registration is refused with both
values in the message.
- **Memory was measured, S3 against local.** Writing and reading the same
Parquet file through `s3://` costs about 15 MiB of resident memory more
than
through `file://` -- and that difference stays flat as the object grows
(7.9 MiB for a 2 MB file, 14.8 MiB for a 149 MB one), so neither the
response
body nor the multipart buffer accumulates.
- **The credential chain was exercised.** With no user mapping the same
statement fails; with `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` in
the
postmaster's environment it succeeds.
- **Nothing new is exported.** `nm -D` lists the PostgreSQL entry points, one
registration function and the test extension's UDFs; no Arrow and no AWS
symbols, on all three build variants.
### Impact
**Dependencies:** an optional build dependency on the AWS SDK for C++.
Without
it the extension builds as before and says so, and opening an `s3://`
location
reports that it was built without it; naming a prefix that has no SDK in it
is
an error rather than a silent fallback. The module is off by default and not
in
the RPM, so packaging is unchanged.
CI builds the SDK from source once per distribution and architecture and
caches
it, starts SeaweedFS for the s3 cases, and now takes Arrow from the Arrow
project's own repository on Rocky 9 and 10 as well, pinned -- 17.0.0 and
21.0.0, so a version change in EPEL cannot arrive without a commit.
What that costs the `ic-datalake-fdw` job, measured on all three legs:
building
the SDK takes 175-228 s on a cache miss and the cache restores in 1 s (it is
4 MB); SeaweedFS is up 6 s after the step starts; the s3 cases add about 35
s,
of which 22 s is one deliberate connect timeout.
**User-facing changes:** a volume's `base_path` now accepts `file://` as well
as `s3://`, and the scheme it rejects is named against what is registered
rather than against a fixed list. Three `iceberg_am_reject` DETAIL lines and
two ERROR lines change wording, the latter because they used to quote back a
URI that can carry a password.
### Checklist
- [x] Followed [contribution
guide](https://cloudberry.apache.org/contribute/code)
- [x] Added/updated documentation
- [x] Reviewed code for security implications
- [ ] This PR contains AI-assisted code generation
- [ ] Requested review from [cloudberry
committers](https://github.com/orgs/apache/teams/cloudberry-committers)
`contrib/datalake_fdw/README.md` documents volumes, where credentials come
from, building with S3 support, and writing a backend -- the example in it is
compiled against the installed headers rather than written out by hand. The
test extension's new functions name a path on the server's file system and
are
revoked from `PUBLIC` like the two that were already there.
### Additional Context
Known limits, deliberately rather than by oversight:
- **A `file://` volume must be the same directory on every host.** Nothing
checks it; the README says so.
- **Between checking that a name is free and creating it, another writer can
take it.** Iceberg's file names are unique by construction, so it does not
arise there.
- **An upload abandoned by a crashed backend leaves its parts behind.** A
bucket lifecycle rule that expires incomplete multipart uploads is the
usual
answer; the README says so.
- **About 15 MiB per backend process is outside `gp_vmem_protect_limit`** --
the SDK client, its connection and one part buffer are allocated by the SDK
rather than through the tracked pool.
- **The fingerprint catches the mismatches that occur in practice**, not
every
possible one.
- **No HDFS.** The scheme is gone from the parser rather than half-supported;
it returns as a backend when someone needs it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
