You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/05/16 07:17:49 UTC
[GitHub] [hive] maheshk114 commented on a diff in pull request #3282: HIVE-26225: Delete operations in ObjectStore.cleanWriteNotificationEvents should be performed in different transactions
maheshk114 commented on code in PR #3282:
URL: https://github.com/apache/hive/pull/3282#discussion_r873347535
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -11766,71 +11720,77 @@ public void cleanNotificationEvents(int olderThan) {
final Optional<Integer> batchSize = (eventBatchSize > 0) ? Optional.of(eventBatchSize) : Optional.empty();
final long start = System.nanoTime();
- int deleteCount = doCleanNotificationEvents(tooOld, batchSize);
+ int deleteCount = doCleanOlderEvents(tooOld, batchSize, table, tableName);
if (deleteCount == 0) {
- LOG.info("No Notification events found to be cleaned with eventTime < {}", tooOld);
+ LOG.info("No {} events found to be cleaned with eventTime < {}", tableName, tooOld);
} else {
int batchCount = 0;
do {
- batchCount = doCleanNotificationEvents(tooOld, batchSize);
+ batchCount = doCleanOlderEvents(tooOld, batchSize, table, tableName);
deleteCount += batchCount;
} while (batchCount > 0);
}
final long finish = System.nanoTime();
- LOG.info("Deleted {} notification events older than epoch:{} in {}ms", deleteCount, tooOld,
- TimeUnit.NANOSECONDS.toMillis(finish - start));
+ LOG.info("Deleted {} {} events older than epoch:{} in {}ms", deleteCount, tableName, tooOld,
+ TimeUnit.NANOSECONDS.toMillis(finish - start));
}
- private int doCleanNotificationEvents(final int ageSec, final Optional<Integer> batchSize) {
Review Comment:
No need to change the name. As its notification only.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -11766,71 +11720,77 @@ public void cleanNotificationEvents(int olderThan) {
final Optional<Integer> batchSize = (eventBatchSize > 0) ? Optional.of(eventBatchSize) : Optional.empty();
final long start = System.nanoTime();
- int deleteCount = doCleanNotificationEvents(tooOld, batchSize);
+ int deleteCount = doCleanOlderEvents(tooOld, batchSize, table, tableName);
if (deleteCount == 0) {
- LOG.info("No Notification events found to be cleaned with eventTime < {}", tooOld);
+ LOG.info("No {} events found to be cleaned with eventTime < {}", tableName, tooOld);
} else {
int batchCount = 0;
do {
- batchCount = doCleanNotificationEvents(tooOld, batchSize);
+ batchCount = doCleanOlderEvents(tooOld, batchSize, table, tableName);
deleteCount += batchCount;
} while (batchCount > 0);
}
final long finish = System.nanoTime();
- LOG.info("Deleted {} notification events older than epoch:{} in {}ms", deleteCount, tooOld,
- TimeUnit.NANOSECONDS.toMillis(finish - start));
+ LOG.info("Deleted {} {} events older than epoch:{} in {}ms", deleteCount, tableName, tooOld,
+ TimeUnit.NANOSECONDS.toMillis(finish - start));
}
- private int doCleanNotificationEvents(final int ageSec, final Optional<Integer> batchSize) {
+ private <T> int doCleanOlderEvents(final int ageSec, final Optional<Integer> batchSize, Class<T> tableClass, String tableName) {
final Transaction tx = pm.currentTransaction();
int eventsCount = 0;
try {
+ String key = null;
tx.begin();
- try (Query query = pm.newQuery(MNotificationLog.class, "eventTime <= tooOld")) {
+ try (Query query = pm.newQuery(tableClass, "eventTime <= tooOld")) {
query.declareParameters("java.lang.Integer tooOld");
- query.setOrdering("eventId ascending");
+ if (MNotificationLog.class.equals(tableClass)) {
+ key = "eventId";
+ } else if (MTxnWriteNotificationLog.class.equals(tableClass)) {
+ key = "txnId";
+ }
+ query.setOrdering(key + " ascending");
if (batchSize.isPresent()) {
query.setRange(0, batchSize.get());
}
- List<MNotificationLog> events = (List) query.execute(ageSec);
+ List<T> events = (List) query.execute(ageSec);
if (CollectionUtils.isNotEmpty(events)) {
eventsCount = events.size();
-
- if (LOG.isDebugEnabled()) {
- int minEventTime, maxEventTime;
- long minEventId, maxEventId;
- Iterator<MNotificationLog> iter = events.iterator();
- MNotificationLog firstNotification = iter.next();
-
- minEventTime = maxEventTime = firstNotification.getEventTime();
- minEventId = maxEventId = firstNotification.getEventId();
-
- while (iter.hasNext()) {
- MNotificationLog notification = iter.next();
- minEventTime = Math.min(minEventTime, notification.getEventTime());
- maxEventTime = Math.max(maxEventTime, notification.getEventTime());
- minEventId = Math.min(minEventId, notification.getEventId());
- maxEventId = Math.max(maxEventId, notification.getEventId());
- }
-
- LOG.debug(
- "Remove notification batch of {} events with eventTime < {}, min eventId {}, max eventId {}, min eventTime {}, max eventTime {}",
- eventsCount, ageSec, minEventId, maxEventId, minEventTime, maxEventTime);
+ int minEventTime, maxEventTime;
Review Comment:
It should be within if (LOG.isDebugEnabled()) {
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -11766,71 +11720,77 @@ public void cleanNotificationEvents(int olderThan) {
final Optional<Integer> batchSize = (eventBatchSize > 0) ? Optional.of(eventBatchSize) : Optional.empty();
final long start = System.nanoTime();
- int deleteCount = doCleanNotificationEvents(tooOld, batchSize);
+ int deleteCount = doCleanOlderEvents(tooOld, batchSize, table, tableName);
if (deleteCount == 0) {
- LOG.info("No Notification events found to be cleaned with eventTime < {}", tooOld);
+ LOG.info("No {} events found to be cleaned with eventTime < {}", tableName, tooOld);
} else {
int batchCount = 0;
do {
- batchCount = doCleanNotificationEvents(tooOld, batchSize);
+ batchCount = doCleanOlderEvents(tooOld, batchSize, table, tableName);
deleteCount += batchCount;
} while (batchCount > 0);
}
final long finish = System.nanoTime();
- LOG.info("Deleted {} notification events older than epoch:{} in {}ms", deleteCount, tooOld,
- TimeUnit.NANOSECONDS.toMillis(finish - start));
+ LOG.info("Deleted {} {} events older than epoch:{} in {}ms", deleteCount, tableName, tooOld,
+ TimeUnit.NANOSECONDS.toMillis(finish - start));
}
- private int doCleanNotificationEvents(final int ageSec, final Optional<Integer> batchSize) {
+ private <T> int doCleanOlderEvents(final int ageSec, final Optional<Integer> batchSize, Class<T> tableClass, String tableName) {
final Transaction tx = pm.currentTransaction();
int eventsCount = 0;
try {
+ String key = null;
tx.begin();
- try (Query query = pm.newQuery(MNotificationLog.class, "eventTime <= tooOld")) {
+ try (Query query = pm.newQuery(tableClass, "eventTime <= tooOld")) {
query.declareParameters("java.lang.Integer tooOld");
- query.setOrdering("eventId ascending");
+ if (MNotificationLog.class.equals(tableClass)) {
+ key = "eventId";
+ } else if (MTxnWriteNotificationLog.class.equals(tableClass)) {
+ key = "txnId";
+ }
+ query.setOrdering(key + " ascending");
if (batchSize.isPresent()) {
query.setRange(0, batchSize.get());
}
- List<MNotificationLog> events = (List) query.execute(ageSec);
+ List<T> events = (List) query.execute(ageSec);
if (CollectionUtils.isNotEmpty(events)) {
eventsCount = events.size();
-
- if (LOG.isDebugEnabled()) {
- int minEventTime, maxEventTime;
- long minEventId, maxEventId;
- Iterator<MNotificationLog> iter = events.iterator();
- MNotificationLog firstNotification = iter.next();
-
- minEventTime = maxEventTime = firstNotification.getEventTime();
- minEventId = maxEventId = firstNotification.getEventId();
-
- while (iter.hasNext()) {
- MNotificationLog notification = iter.next();
- minEventTime = Math.min(minEventTime, notification.getEventTime());
- maxEventTime = Math.max(maxEventTime, notification.getEventTime());
- minEventId = Math.min(minEventId, notification.getEventId());
- maxEventId = Math.max(maxEventId, notification.getEventId());
- }
-
- LOG.debug(
- "Remove notification batch of {} events with eventTime < {}, min eventId {}, max eventId {}, min eventTime {}, max eventTime {}",
- eventsCount, ageSec, minEventId, maxEventId, minEventTime, maxEventTime);
+ int minEventTime, maxEventTime;
+ long minEventId, maxEventId;
+ T firstNotification = events.get(0);
+ T lastNotification = events.get(eventsCount - 1);
+ if (MNotificationLog.class.equals(tableClass)) {
+ minEventTime = ((MNotificationLog)firstNotification).getEventTime();
+ minEventId = ((MNotificationLog)firstNotification).getEventId();
+ maxEventTime = ((MNotificationLog)lastNotification).getEventTime();
+ maxEventId = ((MNotificationLog)lastNotification).getEventId();
Review Comment:
This should be within the loop while (iter.hasNext()) {
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org