justinmclean commented on code in PR #10619:
URL: https://github.com/apache/gravitino/pull/10619#discussion_r3019255692
##########
core/src/main/java/org/apache/gravitino/stats/storage/LancePartitionStatisticStorage.java:
##########
@@ -350,12 +350,24 @@ private void dropStatisticsImpl(Long tableId,
List<PartitionStatisticsDrop> drop
@Override
public void close() throws IOException {
+ datasetCache.ifPresent(
+ cache -> {
+ cache
+ .asMap()
+ .values()
+ .forEach(
+ dataset -> {
+ if (dataset != null) {
+ dataset.close();
+ }
+ });
+ cache.invalidateAll();
+ });
+
if (allocator != null) {
allocator.close();
}
Review Comment:
This works, but for resource cleanup I’d suggest keeping it more
straightforward. The combination of Optional.ifPresent, lambdas, and forEach
makes it a bit harder to follow than a simple loop.
In close() methods, clarity and predictability are usually more important
than conciseness.
Also worth considering: if one dataset.close() throws, the remaining
datasets (and allocator) won’t be closed. We may want to handle that more
defensively.
--
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]
