zddr commented on code in PR #21589:
URL: https://github.com/apache/doris/pull/21589#discussion_r1259036448
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hive/event/AlterDatabaseEvent.java:
##########
@@ -15,34 +15,99 @@
// specific language governing permissions and limitations
// under the License.
-
package org.apache.doris.datasource.hive.event;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.datasource.CatalogIf;
+import org.apache.doris.datasource.ExternalCatalog;
+
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
+import org.apache.hadoop.hive.metastore.api.Database;
import org.apache.hadoop.hive.metastore.api.NotificationEvent;
+import
org.apache.hadoop.hive.metastore.messaging.json.JSONAlterDatabaseMessage;
import java.util.List;
/**
- * MetastoreEvent for Alter_DATABASE event type
+ * MetastoreEvent for ALTER_DATABASE event type
*/
public class AlterDatabaseEvent extends MetastoreEvent {
+ private final Database dbBefore;
+ private final Database dbAfter;
+
+ // true if this alter event was due to a rename operation
+ private final boolean isRename;
+
+ // for test
+ public AlterDatabaseEvent(long eventId, String catalogName, String dbName,
boolean isRename) {
+ super(eventId, catalogName, dbName, null);
+ this.isRename = isRename;
+ this.dbBefore = null;
+ this.dbAfter = null;
+ }
+
private AlterDatabaseEvent(NotificationEvent event,
String catalogName) {
super(event, catalogName);
Preconditions.checkArgument(getEventType().equals(MetastoreEventType.ALTER_DATABASE));
+
+ try {
+ JSONAlterDatabaseMessage alterDatabaseMessage =
+ (JSONAlterDatabaseMessage)
MetastoreEventsProcessor.getMessageDeserializer(event.getMessageFormat())
+ .getAlterDatabaseMessage(event.getMessage());
+ dbBefore =
Preconditions.checkNotNull(alterDatabaseMessage.getDbObjBefore());
+ dbAfter =
Preconditions.checkNotNull(alterDatabaseMessage.getDbObjAfter());
+ } catch (Exception e) {
+ throw new MetastoreNotificationException(
+ debugString("Unable to parse the alter database message"),
e);
+ }
+ // this is a rename event if either dbName of before and after object
changed
+ isRename = !dbBefore.getName().equalsIgnoreCase(dbAfter.getName());
+ }
+
+ private void processRename() throws DdlException {
Review Comment:
How to operate on the hive side? IsRename here will be true
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hive/event/AlterTableEvent.java:
##########
@@ -124,4 +142,18 @@ protected void process() throws
MetastoreNotificationException {
debugString("Failed to process event"), e);
}
}
+
+ @Override
+ protected boolean canBeBatched(MetastoreEvent that) {
+ if (!isSameTable(that)) {
+ return false;
+ }
+ if (isRename() || isView()) {
Review Comment:
Here, it is recommended to abstract a method, such as' willRecreateTable '.
Otherwise, when you see the code here, you need to pay attention to the logic
of isRename and isView
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hive/event/MetastoreEventFactory.java:
##########
@@ -68,19 +73,71 @@ public List<MetastoreEvent>
transferNotificationEventToMetastoreEvents(Notificat
List<MetastoreEvent> getMetastoreEvents(List<NotificationEvent> events,
HMSExternalCatalog hmsExternalCatalog) {
List<MetastoreEvent> metastoreEvents = Lists.newArrayList();
+ String catalogName = hmsExternalCatalog.getName();
for (NotificationEvent event : events) {
-
metastoreEvents.addAll(transferNotificationEventToMetastoreEvents(event,
hmsExternalCatalog.getName()));
+
metastoreEvents.addAll(transferNotificationEventToMetastoreEvents(event,
catalogName));
}
- return createBatchEvents(metastoreEvents);
+ return createBatchEvents(catalogName, metastoreEvents);
}
/**
- * Create batch event tasks according to HivePartitionName to facilitate
subsequent parallel processing.
- * For ADD_PARTITION and DROP_PARTITION, we directly override any events
before that partition.
- * For a partition, it is meaningless to process any events before the
drop partition.
- */
- List<MetastoreEvent> createBatchEvents(List<MetastoreEvent> events) {
- // now do nothing
- return events;
+ * Merge events to reduce the cost time on event processing, currently
mainly handles MetastoreTableEvent
+ * because merge MetastoreTableEvent is simple and cost-effective.
Review Comment:
Some examples are given in the comments, such as: if there are both drop
table events and add partition events, the add partition event will be ignored
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hive/event/AlterTableEvent.java:
##########
@@ -124,4 +142,18 @@ protected void process() throws
MetastoreNotificationException {
debugString("Failed to process event"), e);
}
}
+
+ @Override
+ protected boolean canBeBatched(MetastoreEvent that) {
+ if (!isSameTable(that)) {
+ return false;
+ }
+ if (isRename() || isView()) {
+ return true;
+ }
+ if (that instanceof AlterTableEvent) {
+ return !((AlterTableEvent) that).isRename() && !((AlterTableEvent)
that).isView();
+ }
Review Comment:
Suggest abstracting partition related classes to determine if it is a
partition type. Otherwise, you may wonder which other types of Table types are
identified, and if it is a create database, why batch can be done (although we
know that database related events will not go here)
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
