tkhurana opened a new pull request, #2639:
URL: https://github.com/apache/phoenix/pull/2639
### What changes were proposed in this pull request?
`MetaDataEndpointImpl.buildTable` holds the SYSTEM.CATALOG region operation
(read lock) while it builds a `PTable`. On that path the TTL resolution helpers
`getTTLFromHierarchy` and `getTTLForParentTable` read SYSTEM.CATALOG through an
HBase **client** scan (`ServerUtil.getHTableForCoprocessorScan`), which, for a
non-split SYSTEM.CATALOG, targets the very region whose lock the outer call
holds.
This PR:
- Adds `getSysCatScanner(key, scan)`, used by both helpers. When the local
SYSTEM.CATALOG region hosts the key, it reads the rows directly from the local
region (`Region.getScanner`, wrapped in a small `RegionScanner` →
`ResultScanner` adapter), the same way `buildTable` reads the table's own rows.
If the region is closing, this read fails immediately with
`NotServingRegionException`, so the outer `getTable` releases the lock right
away.
- Otherwise (splittable SYSTEM.CATALOG with the key in another region) it
scans through a new `ServerUtil.ConnectionType.METADATA_LOOKUP_CONNECTION`. Its
configuration (`ServerUtil.getMetadataConfig`) sets the scanner, operation, RPC
read and meta-lookup timeouts to the new
`phoenix.metadata.server.lookup.timeout.ms` (default 10 s; non-positive values
fall back to the default). Region location lookups retry without checking those
timeouts, so the retries are also limited to the largest count whose client
backoff fits within the timeout (`getRetriesWithinTimeout`, using the larger of
`hbase.client.pause` and `hbase.client.pause.server.overloaded`): 7 retries
with the defaults. Without this, the connection inherits the region server's
multiplied retries (45 by default) and the meta-lookup timeouts (20 min by
default).
- Closes the `Table`/`ResultScanner` the TTL helpers open, which previously
leaked.
### Why are the changes needed?
When SYSTEM.CATALOG is closed (region move, drain, RS restart) while a
`getTable` is resolving TTL for a view without its own TTL, or for an index on
a table without one, the nested client scan gets `NotServingRegionException:
... is closing`. The HBase client treats that as retriable and keeps retrying
with backoff until the next backoff would exceed
`hbase.client.scanner.timeout.period` (60 s by default), which happens after
~48 s. All that time the outer handler holds the region read lock, so
`HRegion.doClose` cannot acquire the write lock. `buildTable` uses
`startRegionOperation()` (`Operation.ANY`), so the close's interrupt of
in-flight operations doesn't apply either.
The result is a ~49 s SYSTEM.CATALOG close on every such move. During that
time all metadata RPCs fail with NSRE, clients retry, and the metadata call
queues fill up (`CallQueueTooBigException`). The stall also comes within ~11 s
of `hbase.regionserver.close.wait.time.ms` (60 s), after which a region server
with `hbase.regionserver.close.wait.abort=true` aborts.
The signature of this stall, seen in a production cluster, is a metadata
handler thread logging a client-side `SocketTimeoutException:
callTimeout=60000, callDuration=~68400 ... SYSTEM.CATALOG ... is closing` for
the same region on the same host, immediately followed by `Acquired close lock
on SYSTEM.CATALOG ... after waiting ~48600 ms`. The new IT reproduces exactly
that (`callDuration=68692`, close 49.2 s).
### Does this PR introduce _any_ user-facing change?
- New server-side configuration `phoenix.metadata.server.lookup.timeout.ms`
(default `10000`). It only affects TTL lookups whose rows are in a different
SYSTEM.CATALOG region, i.e. splittable SYSTEM.CATALOG. Such a lookup now fails
sooner against a slow or unavailable remote region (the client's `getTable`
retry handles it) instead of retrying for up to the default client limits.
- Behavior change: closing or moving SYSTEM.CATALOG no longer stalls ~49 s
when a `getTable` is resolving TTL. The TTL values resolved are unchanged.
### How was this patch tested?
New tests:
- `SystemCatalogTTLLookupIT` (own mini-cluster, with the default client
retries and 60 s scanner timeout restored so the old behavior reproduces as in
production):
- `testTableWithIndexesDoesNotScanSystemCatalogViaRpc` and
`testViewHierarchyDoesNotScanSystemCatalogViaRpc` (table → view → view, plus a
view index) assert the resolved TTLs and that no client scan reaches
SYSTEM.CATALOG while the PTables are rebuilt. A `preScannerOpen` observer
counts them; it fires only on the RPC path. A positive control checks the
observer counts a direct client scan.
- `testSystemCatalogCloseIsNotStalledByTTLLookup` closes SYSTEM.CATALOG
while a `getTable` is resolving TTL and asserts the close takes < 10 s.
- `SystemCatalogTTLLookupSplitIT` (extends `SplitSystemCatalogIT`): tenant
views whose ancestors are in other SYSTEM.CATALOG regions, including a
hierarchy where the first hop is already remote. Asserts that the keys are in
distinct regions, that each view's own (local) rows are not client-scanned,
that the remote ancestors are, and that the views resolve the table's TTL.
- `ServerUtilTest`: `getMetadataConfig` timeouts and retries (defaults,
overrides, larger pause, server-overloaded pause, non-positive timeout),
`getRetriesWithinTimeout` edge cases, and connection type routing.
All the new ITs fail without the fix (close took 49.2 s; 2 and 8 client
scans; split IT 1 instead of 0) and pass with it. Both ITs passed 10
consecutive runs; the SYSTEM.CATALOG close took 187–602 ms.
Existing suites run with the change, all passing: `ViewTTLIT`,
`TTLAsPhoenixTTLIT`, `ViewTTLWithLongViewIndexEnabledIT`,
`ViewTTLNotEnabledIT`, `ConditionalTTLExpressionIT`, `StrictTTLPropertyIT`,
`CDCQueryIT`, `CDCStreamIT`, `CDCStreamCompactionTTLIT`, `ViewIndexIT`,
`TenantSpecificViewIndexIT`, `TenantSpecificViewIndexSaltedIT`,
`ReadOnlyViewOnReadOnlyIT`.
Not in this PR (possible follow-up): the `getTableNoCache` call in
`getTableFromCells` for `PHYSICAL_TABLE` links, which runs only with splittable
SYSTEM.CATALOG, goes through a server-side Phoenix connection and is not
bounded by the new connection.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5.5)
--
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]
