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 2021/12/10 20:47:55 UTC
[GitHub] [hive] vihangk1 opened a new pull request #2864: HIVE-25796: Allow metastore clients to fetch remaining events after cleanup
vihangk1 opened a new pull request #2864:
URL: https://github.com/apache/hive/pull/2864
### What changes were proposed in this pull request?
Metastore client throws an exception when events are cleaned up from the metastore side and the lastEventId which is being requested is too old. However, it is possible that client does not care if the events are in sequence or not. In such a case, client should be able to pass in a flag to ignore throwing the exception and return the remaining events from the server.
### Why are the changes needed?
Introduces a new HMS client API for a use-case which it doesn't handle currently.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Added a new unit test.
--
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
[GitHub] [hive] vihangk1 commented on a change in pull request #2864: HIVE-25796: Allow metastore clients to fetch remaining events after cleanup
Posted by GitBox <gi...@apache.org>.
vihangk1 commented on a change in pull request #2864:
URL: https://github.com/apache/hive/pull/2864#discussion_r770743816
##########
File path: standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
##########
@@ -4201,6 +4202,20 @@ public NotificationEventResponse getNextNotification(long lastEventId, int maxEv
NotificationFilter filter) throws TException {
NotificationEventRequest rqst = new NotificationEventRequest(lastEventId);
rqst.setMaxEvents(maxEvents);
+ return getNextNotificationsInternal(rqst, false, filter);
+ }
+
+ @Override
+ public NotificationEventResponse getNextNotification(NotificationEventRequest request,
Review comment:
Actually adding this to the request object will not help since even if the server does the filtering (or throwing the exception) the client API as it is implemented currently will continue to throw the exception. We cannot change the exiting client side API since I believe there are applications (e.g hive replication) which rely on this behavior currently. Hence either ways I think the change will involve client side changes.
--
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
[GitHub] [hive] sourabh912 commented on a change in pull request #2864: HIVE-25796: Allow metastore clients to fetch remaining events after cleanup
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on a change in pull request #2864:
URL: https://github.com/apache/hive/pull/2864#discussion_r771598626
##########
File path: standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
##########
@@ -4201,6 +4202,20 @@ public NotificationEventResponse getNextNotification(long lastEventId, int maxEv
NotificationFilter filter) throws TException {
NotificationEventRequest rqst = new NotificationEventRequest(lastEventId);
rqst.setMaxEvents(maxEvents);
+ return getNextNotificationsInternal(rqst, false, filter);
+ }
+
+ @Override
+ public NotificationEventResponse getNextNotification(NotificationEventRequest request,
Review comment:
That make sense. Thanks for the clarification.
--
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
[GitHub] [hive] vihangk1 commented on pull request #2864: HIVE-25796: Allow metastore clients to fetch remaining events after cleanup
Posted by GitBox <gi...@apache.org>.
vihangk1 commented on pull request #2864:
URL: https://github.com/apache/hive/pull/2864#issuecomment-996942562
Thanks for reviewing Sourabh. Merged into master.
--
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
[GitHub] [hive] sourabh912 commented on a change in pull request #2864: HIVE-25796: Allow metastore clients to fetch remaining events after cleanup
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on a change in pull request #2864:
URL: https://github.com/apache/hive/pull/2864#discussion_r769474574
##########
File path: standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
##########
@@ -4201,6 +4202,20 @@ public NotificationEventResponse getNextNotification(long lastEventId, int maxEv
NotificationFilter filter) throws TException {
NotificationEventRequest rqst = new NotificationEventRequest(lastEventId);
rqst.setMaxEvents(maxEvents);
+ return getNextNotificationsInternal(rqst, false, filter);
+ }
+
+ @Override
+ public NotificationEventResponse getNextNotification(NotificationEventRequest request,
Review comment:
Instead of defining a new getNextNotification api, would it be cleaner if we add `allowGapsInEventIds` as an option in NotificationEventRequest?
--
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
[GitHub] [hive] vihangk1 commented on pull request #2864: HIVE-25796: Allow metastore clients to fetch remaining events after cleanup
Posted by GitBox <gi...@apache.org>.
vihangk1 commented on pull request #2864:
URL: https://github.com/apache/hive/pull/2864#issuecomment-992782724
Looks like the test failures were infra related. I retriggered the run.
--
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
[GitHub] [hive] vihangk1 merged pull request #2864: HIVE-25796: Allow metastore clients to fetch remaining events after cleanup
Posted by GitBox <gi...@apache.org>.
vihangk1 merged pull request #2864:
URL: https://github.com/apache/hive/pull/2864
--
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
[GitHub] [hive] sourabh912 commented on pull request #2864: HIVE-25796: Allow metastore clients to fetch remaining events after cleanup
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on pull request #2864:
URL: https://github.com/apache/hive/pull/2864#issuecomment-996927553
The patch looks good to me !
--
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