stiga-huang commented on code in PR #5688:
URL: https://github.com/apache/hive/pull/5688#discussion_r1986846713


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java:
##########
@@ -11546,66 +11546,64 @@ private void cleanOlderEvents(int olderThan, Class 
table, String tableName) {
       } while (batchCount > 0);
     }
 
-    final long finish = System.nanoTime();
+    final long finish = System.currentTimeMillis();
 
     LOG.info("Deleted {} {} events older than epoch:{} in {}ms", deleteCount, 
tableName, tooOld,
-            TimeUnit.NANOSECONDS.toMillis(finish - start));
+        finish - start);
   }
 
   private <T> int doCleanNotificationEvents(final int ageSec, final 
Optional<Integer> batchSize, Class<T> tableClass, String tableName) {
+    String key;
+    if (MNotificationLog.class.equals(tableClass)) {
+      key = "eventId";
+    } else if (MTxnWriteNotificationLog.class.equals(tableClass)) {
+      key = "txnId";
+    } else {
+      throw new RuntimeException("Cleaning of older " + tableName + " events 
failed. " +
+          "Reason: Unknown table encountered " + tableClass.getName());
+    }
     final Transaction tx = pm.currentTransaction();
     int eventsCount = 0;
 
     try {
-      String key = null;
       tx.begin();
 
-      try (Query query = pm.newQuery(tableClass, "eventTime <= tooOld")) {
+      try (Query query = pm.newQuery(String.format(
+          "select %s, eventTime from %s where eventTime <= tooOld order by %s",

Review Comment:
   > can we try select min(%s), max(%s), min(eventTime), max(eventTime) ... 
   
   We want to delete events using the given batch size. This query with 
`query.setRange(0, batchSize.get())` at L11576 is used to get the n-th event id 
(i.e. maxId) so we can delete events in range [minId, maxId].
   
   Using min(), max() functions, we might need a more complex query like
   ```
   select min(eventId), max(eventId) from (
     select eventId, eventTime from tableName
     where eventTime <= tooOld order by eventId limit 10000);
   ```
   However, it seems JDOQL just supports a simple syntax and doesn't allow this.
   
https://www.datanucleus.org/products/accessplatform_5_2/jdo/query.html#jdoql_string
   
   Got an exception while trying the above query:
   ```
   javax.jdo.JDOUserException: Query contains a JDOQL keyword ("from") that is 
out of order. Keywords can only be used in a defined order.
           at 
org.datanucleus.api.jdo.NucleusJDOHelper.getJDOExceptionForNucleusException(NucleusJDOHelper.java:635)
 ~[datanucleus-api-jdo-5.2.8.jar:?]
           at 
org.datanucleus.api.jdo.JDOPersistenceManager.newQuery(JDOPersistenceManager.java:1309)
 ~[datanucleus-api-jdo-5.2.8.jar:?]
           at 
org.datanucleus.api.jdo.JDOPersistenceManager.newQuery(JDOPersistenceManager.java:1247)
 ~[datanucleus-api-jdo-5.2.8.jar:?]
           at 
org.apache.hadoop.hive.metastore.ObjectStore.doCleanNotificationEvents(ObjectStore.java:12210)
 ~[hive-standalone-metastore-3.1.3000.7.3.1.0-160.jar:3.1.3000.7.3.1.0-SNAPSHOT]
           at 
org.apache.hadoop.hive.metastore.ObjectStore.cleanOlderEvents(ObjectStore.java:12176)
 ~[hive-standalone-metastore-3.1.3000.7.3.1.0-160.jar:3.1.3000.7.3.1.0-SNAPSHOT]
           at 
org.apache.hadoop.hive.metastore.ObjectStore.cleanNotificationEvents(ObjectStore.java:12162)
 ~[hive-standalone-metastore-3.1.3000.7.3.1.0-160.jar:3.1.3000.7.3.1.0-SNAPSHOT]
           at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) 
~[?:1.8.0_432]
           at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) 
~[?:1.8.0_432]
           at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
 ~[?:1.8.0_432]
           at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_432]
           at 
org.apache.hadoop.hive.metastore.RawStoreProxy.invoke(RawStoreProxy.java:97) 
~[hive-standalone-metastore-3.1.3000.7.3.1.0-160.jar:3.1.3000.7.3.1.0-SNAPSHOT]
           at com.sun.proxy.$Proxy33.cleanNotificationEvents(Unknown Source) 
~[?:?]
           at 
org.apache.hive.hcatalog.listener.DbNotificationListener$CleanerThread.run(DbNotificationListener.java:1354)
 
~[hive-hcatalog-server-extensions-3.1.3000.7.3.1.0-160.jar:3.1.3000.7.3.1.0-160]
   Caused by: org.datanucleus.exceptions.NucleusUserException: Query contains a 
JDOQL keyword ("from") that is out of order. Keywords can only be used in a 
defined order.
           at 
org.datanucleus.query.JDOQLSingleStringParser$Compiler.compile(JDOQLSingleStringParser.java:124)
 ~[datanucleus-core-5.2.10.jar:?]
           at 
org.datanucleus.query.JDOQLSingleStringParser$Compiler.access$000(JDOQLSingleStringParser.java:108)
 ~[datanucleus-core-5.2.10.jar:?]
           at 
org.datanucleus.query.JDOQLSingleStringParser.parse(JDOQLSingleStringParser.java:100)
 ~[datanucleus-core-5.2.10.jar:?]
           at 
org.datanucleus.store.query.AbstractJDOQLQuery.<init>(AbstractJDOQLQuery.java:122)
 ~[datanucleus-core-5.2.10.jar:?]
           at 
org.datanucleus.store.rdbms.query.JDOQLQuery.<init>(JDOQLQuery.java:159) 
~[datanucleus-rdbms-5.2.10.jar:?]
           at 
org.datanucleus.store.rdbms.RDBMSStoreManager.newQuery(RDBMSStoreManager.java:1192)
 ~[datanucleus-rdbms-5.2.10.jar:?]
           at 
org.datanucleus.api.jdo.JDOPersistenceManager.newQuery(JDOPersistenceManager.java:1300)
 ~[datanucleus-api-jdo-5.2.8.jar:?]
           ... 11 more
   ```



-- 
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-unsubscr...@hive.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

Reply via email to