You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/03/11 22:02:09 UTC
[GitHub] [incubator-pinot] mcvsubbu opened a new issue #6671: RealtimeTableDataManager shuts down SegmentBuildTimeLeaseExtender for all tables in the host
mcvsubbu opened a new issue #6671:
URL: https://github.com/apache/incubator-pinot/issues/6671
This bug has surfaced because of recent commit PR #6608 . The problem is that the lease extender is a common one for all tables, and we cannot shut it down until the system is being shut down.
@Jackie-Jiang
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] snleee edited a comment on issue #6671: RealtimeTableDataManager shuts down SegmentBuildTimeLeaseExtender for all tables in the host
Posted by GitBox <gi...@apache.org>.
snleee edited a comment on issue #6671:
URL: https://github.com/apache/incubator-pinot/issues/6671#issuecomment-799623646
1. The routing back to the proper leader controller has been happening. `ServerSegmentCompletionProtocolHandler:createSegmentCompletionUrl()`.
2. The main issue was that the server-side only had the lease extender per instance id, so each server has had 1 single global lease extender. `shutdown()` call from a single data manager affected the lease extender for other data managers.
3. @jtao15's fix changes servers to have lease extender per table. I think that this is cleaner than having one global lease extender. (1-1 mapping for lease extender - data manager for a table).
@mcvsubbu How do you think? Can you also review the above PR?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] snleee edited a comment on issue #6671: RealtimeTableDataManager shuts down SegmentBuildTimeLeaseExtender for all tables in the host
Posted by GitBox <gi...@apache.org>.
snleee edited a comment on issue #6671:
URL: https://github.com/apache/incubator-pinot/issues/6671#issuecomment-799623646
1. The routing back to the proper leader controller has been happening. `ServerSegmentCompletionProtocolHandler:createSegmentCompletionUrl()`.
2. The main issue was that the server-side only had the lease extender per instance id, so each server has had 1 single global lease extender. `shutdown()` call from a single data manager affected the lease extender for other data managers.
3. @jtao15's fix changes servers to have lease extender per table. I think that this is cleaner and even simpler than having one global lease extender (1-1 mapping for lease extender - data manager for a table). It turns out that `ServerSegmentCompletionProtocolHandler` needs the table name so we actually need to instantiate the lease extender with the table name.
@mcvsubbu How do you think? Can you also review the above PR?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on issue #6671: RealtimeTableDataManager shuts down SegmentBuildTimeLeaseExtender for all tables in the host
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on issue #6671:
URL: https://github.com/apache/incubator-pinot/issues/6671#issuecomment-797163409
Looks like leases have not been working right since PR #4323 on multi-tenant realtime clusters. no effect on single-tenant clusters.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on issue #6671: RealtimeTableDataManager shuts down SegmentBuildTimeLeaseExtender for all tables in the host
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on issue #6671:
URL: https://github.com/apache/incubator-pinot/issues/6671#issuecomment-797732964
cc: @snleee , @sajjad-moradi
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on issue #6671: RealtimeTableDataManager shuts down SegmentBuildTimeLeaseExtender for all tables in the host
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on issue #6671:
URL: https://github.com/apache/incubator-pinot/issues/6671#issuecomment-799773072
@snleee The existing implementation may not route to the correct controller in multi-tenant use cases. If you notice, the constructor of lease extender takes a table name, and that table name is used to find the controller to talk to.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] snleee commented on issue #6671: RealtimeTableDataManager shuts down SegmentBuildTimeLeaseExtender for all tables in the host
Posted by GitBox <gi...@apache.org>.
snleee commented on issue #6671:
URL: https://github.com/apache/incubator-pinot/issues/6671#issuecomment-799623646
1. The routing back to the proper leader controller has been happening. `ServerSegmentCompletionProtocolHandler:createSegmentCompletionUrl()`.
2. The main issue was that the server-side only had the lease extender per instance id, which means that 1 single global lease extender.
3. @jtao15's fix changes servers to have lease extender per table. I think that this is cleaner than having one global lease extender. (1-1 mapping for lease extender - data manager for a table).
@mcvsubbu How do you think? Can you also review the above PR?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] snleee edited a comment on issue #6671: RealtimeTableDataManager shuts down SegmentBuildTimeLeaseExtender for all tables in the host
Posted by GitBox <gi...@apache.org>.
snleee edited a comment on issue #6671:
URL: https://github.com/apache/incubator-pinot/issues/6671#issuecomment-799623646
1. The routing back to the proper leader controller has been happening. `ServerSegmentCompletionProtocolHandler:createSegmentCompletionUrl()`.
2. The main issue was that the server-side only had the lease extender per instance id, which means that 1 single global lease extender and shutdown() call from a single data manager affected to the lease extender for other data managers.
3. @jtao15's fix changes servers to have lease extender per table. I think that this is cleaner than having one global lease extender. (1-1 mapping for lease extender - data manager for a table).
@mcvsubbu How do you think? Can you also review the above PR?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] snleee closed issue #6671: RealtimeTableDataManager shuts down SegmentBuildTimeLeaseExtender for all tables in the host
Posted by GitBox <gi...@apache.org>.
snleee closed issue #6671:
URL: https://github.com/apache/incubator-pinot/issues/6671
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on issue #6671: RealtimeTableDataManager shuts down SegmentBuildTimeLeaseExtender for all tables in the host
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on issue #6671:
URL: https://github.com/apache/incubator-pinot/issues/6671#issuecomment-797162771
There are other issues with the lease extender as well, as pointed out by @Jackie-Jiang . Looks like things changed during controller distribution and the lease extender was not fixed right.
Overall, here is what needs to happen:
- Lease extended should get instantiated in HelixServerStarter instead of table data manager. It should also get stopped there. It should be instantiated without a table name , of course.
- The addSegment API of lease extender should be called with an addition argument of protocol handler , so that the extend lease request goes to the right controller for the table.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] snleee edited a comment on issue #6671: RealtimeTableDataManager shuts down SegmentBuildTimeLeaseExtender for all tables in the host
Posted by GitBox <gi...@apache.org>.
snleee edited a comment on issue #6671:
URL: https://github.com/apache/incubator-pinot/issues/6671#issuecomment-799623646
1. The routing back to the proper leader controller has been happening. `ServerSegmentCompletionProtocolHandler:createSegmentCompletionUrl()`.
2. The main issue was that the server-side only had the lease extender per instance id, so each server has had 1 single global lease extender. `shutdown()` call from a single data manager affected the lease extender for other data managers.
3. @jtao15's fix changes servers to have lease extender per table. I think that this is cleaner and even simpler than having one global lease extender (1-1 mapping for lease extender - data manager for a table). `ServerSegmentCompletionProtocolHandler` needs the table name so we actually need to instantiate the lease extender with the table name.
@mcvsubbu How do you think? Can you also review the above PR?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] snleee commented on issue #6671: RealtimeTableDataManager shuts down SegmentBuildTimeLeaseExtender for all tables in the host
Posted by GitBox <gi...@apache.org>.
snleee commented on issue #6671:
URL: https://github.com/apache/incubator-pinot/issues/6671#issuecomment-799799483
@mcvsubbu You're right. I got confused. It keeps using the same lease extender since it looks up based on the instanceId
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org