PDGGK commented on PR #113:
URL: https://github.com/apache/iotdb-extras/pull/113#issuecomment-4912439640
Thanks for the thorough review, @CritasWang — all five addressed in 511f9bf.
**1. Duplicated bounded executor across the three DAOs
(`IoTDBTableLatestDao.java:816`).** Lifted the whole mechanism —
`initReadExecutor` / `submitReadTask` / `ReadTask` / `failDroppedReadTask` and
the shutdown-drain — into `IoTDBTableBaseDao`; the three DAOs now delegate, so
a future shutdown-race fix lives in one place. Attributes' `submitIoTask` /
`IoTask` / `ioExecutor` collapse onto the shared `submitReadTask`. The
semantics you'd want preserved are unchanged: bounded `ArrayBlockingQueue` +
`AbortPolicy`, per-DAO thread-name prefix, the
`IoTDBTableReadQueueFullException` / `IoTDBTableDaoShuttingDownException`
types, and the `accepting=false → shutdownNow → drain` ordering.
**2. Duplicated small utilities (`IoTDBTableLatestDao.java:806`).**
`sqlString` (the single SQL single-quote escape point you flagged as the
highest-risk to have scattered), `kvEntry`, `requireTelemetryKey` /
`requireKey`, and `entityPredicate` are now single copies in the base and
inherited by the DAOs.
**3. Attributes executor borrowed `iotdb.ts.read.*`
(`IoTDBTableAttributesDao.java:251`).** Added a dedicated
`iotdb.attributes.executor.threads` / `.queue-capacity` block (defaults
identical to `iotdb.ts.read`, so no behavior change unless tuned); the
attributes DAO now sizes from it, so its concurrency / backpressure can be
tuned independently of the timeseries selector.
**4. Write path shares the read executor + "read"-named queue-full exception
(`IoTDBTableLatestDao.java:298`).** I took the documentation path you offered
rather than splitting the pools: a dedicated latest-overlay write executor
would sit almost entirely idle (at most one small task per in-order latest
update), so I kept the shared bounded executor and instead (a) enriched the
`IoTDBTableReadQueueFullException` message to state explicitly that a
`saveLatest` / `removeLatest` write burst can reject a concurrent `findLatest`
/ `findAllLatest`, and (b) documented the shared read+write executor/queue on
`saveLatest` and on the base `initReadExecutor`. If you'd rather have the
isolation, I'm happy to split it into separate read/write pools instead — just
say the word.
**5. Asymmetric cluster gate (`IoTDBTableLatestDao.java:241`).** Added a
symmetric `iotdb.ts_latest.cluster_mode` acknowledgement to the latest overlay,
mirroring the attributes gate exactly (same accepted set `{sticky-routing,
disabled}`, same fail-fast at construction), so the in-JVM `Striped`-lock
cluster caveat is opt-in the same way on both paths. Done in this PR rather
than a follow-up.
Re-verified with the build cache off: 155 unit + 37 Testcontainers IT
(against `apache/iotdb:2.0.8-standalone`) green; checkstyle, spotless,
apache-rat, and enforcer clean.
--
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]
