Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add timeout for storage/meta clients and add some error logs #1399

Merged
merged 3 commits into from
Dec 9, 2019
Merged

Add timeout for storage/meta clients and add some error logs #1399

merged 3 commits into from
Dec 9, 2019

Conversation

dangleptr
Copy link
Contributor

@dangleptr dangleptr commented Dec 9, 2019 •
edited
Loading

It is reported by our users.

After network broken down, the storaged can't connect to metad some times, meanwhile the graphd can't connect to storaged either. Currently, the client will wait for the response without timeout, it is unreasonable.
"timeout" will be helpful to recover from broken connections.

The error logs here is to help our users figure out the problems if failed.

@dangleptr dangleptr added the ready-for-testing PR: ready for the CI test label Dec 9, 2019
@zlcook
Copy link
Contributor

zlcook commented Dec 9, 2019

Can you log the mapping of uuid-value and value ?

@nebula-community-bot
Copy link
Member

Unit testing passed.

@nebula-community-bot
Copy link
Member

Unit testing failed.

@dangleptr
Copy link
Contributor Author

dangleptr commented Dec 9, 2019 •
edited
Loading

Can you log the mapping of uuid-value and value ?

Good point. I have added some error logs for uuid request. It will output the string id.
If you want to know the the partId for the string id, you could calc it by yourself with this command.

YIELD hash("string id") % 1024 + 1

@dangleptr dangleptr mentioned this pull request Dec 9, 2019
Closed
zlcook
zlcook reviewed Dec 9, 2019
View reviewed changes
src/common/filter/Expressions.cpp
<< ", part " << rc.get_part_id() << ", str id " << toString();
return OptVariantType(Status::Error("Get UUID Failed"));
}
return v.get_id();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about vlog the mapping of uuid and value for success ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good

@nebula-community-bot
Copy link
Member

Unit testing passed.

@zlcook
Copy link
Contributor

zlcook commented Dec 9, 2019

Can you log the mapping of uuid-value and value ?

Good point. I have added some error logs for uuid request. It will output the string id.
If you want to know the the partId for the string id, you could calc it by yourself with this command.

YIELD hash("string id") % 1024 + 1

Very good

@Amber1990Zhang
Copy link
Contributor

Can you log the mapping of uuid-value and value ?

Good point. I have added some error logs for uuid request. It will output the string id.
If you want to know the the partId for the string id, you could calc it by yourself with this command.

YIELD hash("string id") % 1024 + 1

Sorry I have a small question here, is 1024 the default partition number? If so, it should be 100 now, shouldn't it?

@zlcook
Copy link
Contributor

zlcook commented Dec 9, 2019

LGTM

@nebula-community-bot
Copy link
Member

Unit testing passed.

darionyaphet
darionyaphet approved these changes Dec 9, 2019
View reviewed changes
Copy link
Contributor

@darionyaphet darionyaphet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good Work ~

liuyu85cn
liuyu85cn approved these changes Dec 9, 2019
View reviewed changes
Copy link
Contributor

@liuyu85cn liuyu85cn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nebula-community-bot
Copy link
Member

Unit testing passed.

@dangleptr dangleptr merged commit 628d069 into vesoft-inc:master Dec 9, 2019
@dangleptr dangleptr deleted the bug3 branch December 9, 2019 10:49
@laura-ding laura-ding mentioned this pull request Dec 17, 2019
Closed
yixinglu pushed a commit to yixinglu/nebula that referenced this pull request Feb 16, 2020
@dangleptr
Add timeout for storage/meta clients and add some error logs (vesoft-…
2edb88c 
…inc#1399)

* Add timeout for storage/meta clients and add some eror logs

* address zlcook's comments
tong-hao pushed a commit to tong-hao/nebula that referenced this pull request Jun 1, 2021
@dangleptr
Add timeout for storage/meta clients and add some error logs (vesoft-…
a1e2112 
…inc#1399)

* Add timeout for storage/meta clients and add some eror logs

* address zlcook's comments
yixinglu pushed a commit to yixinglu/nebula that referenced this pull request Jan 31, 2023
@nebula-bots @jievince
fix map hash method (vesoft-inc#1399)
292f756 
Co-authored-by: jie.wang <38901892+jievince@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zlcook zlcook zlcook left review comments

@darionyaphet darionyaphet darionyaphet approved these changes

@liuyu85cn liuyu85cn liuyu85cn approved these changes

@zhangguoqing zhangguoqing Awaiting requested review from zhangguoqing

@critical27 critical27 Awaiting requested review from critical27

@bright-starry-sky bright-starry-sky Awaiting requested review from bright-starry-sky

Assignees
No one assigned
Labels
ready-for-testing PR: ready for the CI test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@dangleptr @zlcook @nebula-community-bot @Amber1990Zhang @darionyaphet @liuyu85cn