letian-jiang commented on PR #3086: URL: https://github.com/apache/drill/pull/3086#issuecomment-6064305875
Following up on the remaining points: - **3. BIGINT serialization:** Ordinary BIGINT literals now retain their existing serialization format. Bound dynamic parameters preserve their type through the parameter wrapper. I also removed the parser special case that converted BIGINT casts back into literals, so explicit casts retain their normal behavior. - **4. Context fingerprints:** Effective option and storage-plugin configuration fingerprints are now included in the cache key. Different configurations can coexist instead of repeatedly invalidating each other. Table versions remain compatibility checks so incompatible table changes invalidate the cached entry. - **5. Metadata I/O:** The [[benchmark section](https://github.com/apache/drill/pull/3086#benchmark)](https://github.com/apache/drill/pull/3086#benchmark) already compares ordinary planning with confirmed cache hits, including planning time and end-to-end latency. For example, HBase point-read planning drops from 50 to 14 ms, and end-to-end latency from 66 to 29 ms. I’ve also applied the two suggested HBase improvements: removed the separate `tableExists` check and switched to `Connection.getTable(name).getDescriptor()`, avoiding an `Admin` handle per lookup. `TableNotFoundException` returns no cache metadata, while other I/O failures retain the existing fallback behavior. - **6. Trial binding:** My intended contract is that parameter binding should be reliable for every eligible plan. Deterministic binding failures indicate a gap in the binder or eligibility checks that should be fixed there. A trial bind with the first execution’s values can catch some defects, but does not establish correctness for subsequent values. I have retained JSON read-back validation without adding a trial bind. Scan reconstruction can still fail because of metadata or I/O errors; the existing invalidation and replanning path handles those failures. - **7. Writer queue:** The plan is now serialized immediately after planning, before parallelization or execution can mutate its operators. Pending publication and the writer queue retain the immutable JSON string instead of the live physical-plan graph. Publication still happens only after successful execution. - **9. Plan quality:** I agree that this is an expected trade-off. The cached plan must remain general enough for subsequent parameter values, so constant folding based on those values cannot specialize it to one execution. Value-specific selectivity, join ordering and distribution choices may also be less effective, including on the initial miss. This limitation is now documented. - **10. Observability:** Added DEBUG logging for hits, misses and invalidation reasons. The `hits`, `misses`, `invalidations`, `evictions` and `entries` gauges are registered in `DrillMetrics` under `drill.plan_cache.`, using the existing JMX and log reporters. - **11. Cache clearing:** Added `ALTER SYSTEM CLEAR PLAN CACHE`. It clears the receiving Drillbit’s cache and returns its address. With authentication enabled, it follows the existing SYSTEM administrator policy. Clearing also prevents plans prepared before the command, including queued or pending publications, from restoring old entries. - **12. Documentation:** Both documents have been moved to `docs/dev/` and linked from `DevDocs.md`. They now cover configuration, metrics, the clearing command, plan-quality trade-offs and the plugin expansion roadmap. -- 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]
