keith-turner commented on code in PR #5668:
URL: https://github.com/apache/accumulo/pull/5668#discussion_r2183239219
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java:
##########
@@ -232,17 +233,34 @@ public Entry<Key,Value> next() {
};
final Set<ManagementAction> actions = new HashSet<>();
- final TabletMetadata tm =
- TabletMetadata.convertRow(kvIter, TabletManagement.CONFIGURED_COLUMNS,
false, true);
-
Exception error = null;
+ TabletMetadata tm = null;
+ KeyExtent extent = null;
try {
- LOG.trace("Evaluating extent: {}", tm);
- computeTabletManagementActions(tm, actions);
- } catch (Exception e) {
- LOG.error("Error computing tablet management actions for extent: {}",
tm.getExtent(), e);
+ tm = TabletMetadata.convertRow(kvIter,
TabletManagement.CONFIGURED_COLUMNS, false, true);
+ } catch (RuntimeException e) {
+ LOG.error("Failed to convert tablet metadata at row: {}",
+ keys.get(0) == null ? "no key" : keys.get(0).getRow(), e);
Review Comment:
It does not hurt to check for keys[0] being nuill, however looking at
RowEncodingIterator.prepKeys() it does not seem like it would ever pass null
but that could change in future.
Was wondering if RowEncodingIterator.prepKeys() would ever pass an empty
list. Seems like it will not.
##########
server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java:
##########
@@ -488,9 +488,17 @@ private TableMgmtStats
manageTablets(Iterator<TabletManagement> iter,
if (mtiError != null) {
// An error happened on the TabletServer in the
TabletManagementIterator
// when trying to process this extent.
+ KeyExtent ke = null;
+ try {
+ ke = tm.getExtent();
+ } catch (IllegalStateException e) {
+ // thrown when no prev endrow in tablet metadata.
+ // suppress this exception and leave ke set to null
Review Comment:
could log this exception at trace
##########
server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java:
##########
@@ -488,9 +488,17 @@ private TableMgmtStats
manageTablets(Iterator<TabletManagement> iter,
if (mtiError != null) {
// An error happened on the TabletServer in the
TabletManagementIterator
// when trying to process this extent.
+ KeyExtent ke = null;
+ try {
+ ke = tm.getExtent();
+ } catch (IllegalStateException e) {
+ // thrown when no prev endrow in tablet metadata.
+ // suppress this exception and leave ke set to null
+ }
+ String errorLocation = ke == null ? " table: " + tm.getTableId() : "
extent: " + ke;
Review Comment:
On the server side there is now code in TabletManagementIter that handles
when TabletMetadata.convertRow throws an exception. For that case where
happens here on the client side in the manger, will tm be null? Wondering
about the call to `tm.getTableId()` for that case.
--
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]
