Skip to content

update insert pending segments logic to synchronous#6283

Closed
FaxianZhao wants to merge 1 commit intoapache:masterfrom
FaxianZhao:sync_insert_pendingSegments
Closed

update insert pending segments logic to synchronous#6283
FaxianZhao wants to merge 1 commit intoapache:masterfrom
FaxianZhao:sync_insert_pendingSegments

Conversation

@FaxianZhao
Copy link
Copy Markdown
Contributor

@FaxianZhao FaxianZhao commented Aug 31, 2018

Current pending segment id logic is not friendly to my business. It will retry to find the max partition num and plus 1 until reach max retry count.

So I think make this logic TaskLock level synchronous is more smoothly.
Add the SegmentLock.class is my work around solution. Because TaskLock is belong to druid-indexing-service module, it cannot be import to IndexerMetadataStorageCoordinator with druid-server module. Either, add a ReentrantLock in TaskLock, maybe.

If you have some better solution, please let me know.

Thank you.

update sync insert pending segments logic

update sync insert pending segments logic

update sync insert pending segments logic
@FaxianZhao FaxianZhao closed this Sep 11, 2018
@gianm
Copy link
Copy Markdown
Contributor

gianm commented Sep 11, 2018

Hey @FaxianZhao - any reaosn you closed this? I was just thinking about reviewing this. Sorry we didn't get to it straight away. We have a bit of a backlog of pull requests.

@FaxianZhao
Copy link
Copy Markdown
Contributor Author

FaxianZhao commented Sep 12, 2018

Hey @gianm ,I'm sorry for close this without any comment.
I think I have some misunderstanding about the original code logic. This patch cannot fix the issue in my last test.

Now I'm trying to refine it. If I finish it, I will reopen this pull request.

@FaxianZhao FaxianZhao deleted the sync_insert_pendingSegments branch September 14, 2018 08:43
@FaxianZhao
Copy link
Copy Markdown
Contributor Author

Hi, @gianm
I'm sorry, I delete the branch about #6283 by mistake. I can't do anything in the previous branch.
So, please check #6336

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants