Skip to content

Commit 837660b

Browse files
committed
Added logging for exceptions
1 parent 58cfafa commit 837660b

File tree

1 file changed

+75
-66
lines changed

1 file changed

+75
-66
lines changed

java/memory/memory-core/src/main/java/org/apache/arrow/memory/BaseAllocator.java

Lines changed: 75 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -408,89 +408,98 @@ public AllocationReservation newReservation() {
408408

409409
@Override
410410
public synchronized void close() {
411-
/*
412-
* Some owners may close more than once because of complex cleanup and shutdown
413-
* procedures.
414-
*/
415-
if (isClosed) {
416-
return;
417-
}
418-
419-
isClosed = true;
411+
try {
412+
/*
413+
* Some owners may close more than once because of complex cleanup and shutdown
414+
* procedures.
415+
*/
416+
if (isClosed) {
417+
return;
418+
}
420419

421-
StringBuilder outstandingChildAllocators = new StringBuilder();
422-
if (DEBUG) {
423-
synchronized (DEBUG_LOCK) {
424-
verifyAllocator();
420+
isClosed = true;
425421

426-
// are there outstanding child allocators?
427-
if (!childAllocators.isEmpty()) {
428-
for (final BaseAllocator childAllocator : childAllocators.keySet()) {
429-
if (childAllocator.isClosed) {
430-
logger.warn(String.format(
431-
"Closed child allocator[%s] on parent allocator[%s]'s child list.\n%s",
432-
childAllocator.name, name, toString()));
422+
StringBuilder outstandingChildAllocators = new StringBuilder();
423+
if (DEBUG) {
424+
synchronized (DEBUG_LOCK) {
425+
verifyAllocator();
426+
427+
// are there outstanding child allocators?
428+
if (!childAllocators.isEmpty()) {
429+
for (final BaseAllocator childAllocator : childAllocators.keySet()) {
430+
if (childAllocator.isClosed) {
431+
logger.warn(String.format(
432+
"Closed child allocator[%s] on parent allocator[%s]'s child list.\n%s",
433+
childAllocator.name, name, toString()));
434+
}
433435
}
436+
437+
throw new IllegalStateException(
438+
String.format("Allocator[%s] closed with outstanding child allocators.\n%s", name,
439+
toString()));
434440
}
435441

436-
throw new IllegalStateException(
437-
String.format("Allocator[%s] closed with outstanding child allocators.\n%s", name,
438-
toString()));
439-
}
442+
// are there outstanding buffers?
443+
final int allocatedCount = childLedgers.size();
444+
if (allocatedCount > 0) {
445+
throw new IllegalStateException(
446+
String.format("Allocator[%s] closed with outstanding buffers allocated (%d).\n%s",
447+
name, allocatedCount, toString()));
448+
}
440449

441-
// are there outstanding buffers?
442-
final int allocatedCount = childLedgers.size();
443-
if (allocatedCount > 0) {
444-
throw new IllegalStateException(
445-
String.format("Allocator[%s] closed with outstanding buffers allocated (%d).\n%s",
446-
name, allocatedCount, toString()));
447-
}
450+
if (reservations.size() != 0) {
451+
throw new IllegalStateException(
452+
String.format("Allocator[%s] closed with outstanding reservations (%d).\n%s", name,
453+
reservations.size(),
454+
toString()));
455+
}
448456

449-
if (reservations.size() != 0) {
450-
throw new IllegalStateException(
451-
String.format("Allocator[%s] closed with outstanding reservations (%d).\n%s", name,
452-
reservations.size(),
453-
toString()));
454457
}
455-
456-
}
457-
} else {
458-
if (!childAllocators.isEmpty()) {
459-
outstandingChildAllocators.append("Outstanding child allocators : \n");
460-
synchronized (childAllocators) {
461-
for (final BaseAllocator childAllocator : childAllocators.keySet()) {
462-
outstandingChildAllocators.append(String.format(" %s", childAllocator.toString()));
458+
} else {
459+
if (!childAllocators.isEmpty()) {
460+
outstandingChildAllocators.append("Outstanding child allocators : \n");
461+
synchronized (childAllocators) {
462+
for (final BaseAllocator childAllocator : childAllocators.keySet()) {
463+
outstandingChildAllocators.append(String.format(" %s", childAllocator.toString()));
464+
}
463465
}
464466
}
465467
}
466-
}
467468

468-
// Is there unaccounted-for outstanding allocation?
469-
final long allocated = getAllocatedMemory();
470-
if (allocated > 0) {
471-
if (parent != null && reservation > allocated) {
472-
parent.releaseBytes(reservation - allocated);
469+
// Is there unaccounted-for outstanding allocation?
470+
final long allocated = getAllocatedMemory();
471+
if (allocated > 0) {
472+
if (parent != null && reservation > allocated) {
473+
parent.releaseBytes(reservation - allocated);
474+
}
475+
String msg = String.format("Memory was leaked by query. Memory leaked: (%d)\n%s%s", allocated,
476+
outstandingChildAllocators.toString(), toString());
477+
logger.error(msg);
478+
479+
throw new IllegalStateException(msg);
473480
}
474-
String msg = String.format("Memory was leaked by query. Memory leaked: (%d)\n%s%s", allocated,
475-
outstandingChildAllocators.toString(), toString());
476-
logger.error(msg);
477-
throw new IllegalStateException(msg);
478-
}
479481

480-
// we need to release our memory to our parent before we tell it we've closed.
481-
super.close();
482+
// we need to release our memory to our parent before we tell it we've closed.
483+
super.close();
482484

483-
// Inform our parent allocator that we've closed
484-
if (parentAllocator != null) {
485-
parentAllocator.childClosed(this);
486-
}
485+
// Inform our parent allocator that we've closed
486+
if (parentAllocator != null) {
487+
parentAllocator.childClosed(this);
488+
}
487489

488-
if (DEBUG) {
489-
historicalLog.recordEvent("closed");
490-
logger.debug(String.format("closed allocator[%s].", name));
490+
if (DEBUG) {
491+
historicalLog.recordEvent("closed");
492+
logger.debug(String.format("closed allocator[%s].", name));
493+
}
494+
} catch (Exception e) {
495+
logger.warn(
496+
"BufferAllocator.close() of {} got an exception {} - Details {}",
497+
this,
498+
e,
499+
this.toVerboseString());
500+
501+
throw e;
491502
}
492-
493-
494503
}
495504

496505
@Override

0 commit comments

Comments
 (0)