-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Acquire rs readers at the beginning of the olapscanner #1400
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.
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
Conversation
| // if not clear rowset readers in read_params here | ||
| // readers will be release when runtime state deconstructed but | ||
| // deconstructor in reader references runtime state | ||
| // so that it will core |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment make me confusing...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It means, we should deconstructor resource in close method. Not in deconstructor method.
be/src/olap/reader.cpp
Outdated
| || read_params.reader_type == READER_QUERY) { | ||
| rs_readers = &read_params.rs_readers; | ||
| } else { | ||
| _tablet->obtain_header_rdlock(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this branch be deleted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could not. because there is another reader_type CHECK_SUM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can rowsets be aquired in the same method as other types for CHECK_SUM?
Or it looks odd.
| */ | ||
| // TODO(ygl): temp modify it for test !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! | ||
| @ConfField(mutable = true, masterOnly = true) | ||
| public static long catalog_trash_expire_second = 10L; // 1day |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can do test by using modify configuration file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the past, it is one day。。。。
be/src/exec/olap_scanner.cpp
Outdated
| // the rowsets maybe compacted when the last olap scanner starts | ||
| _tablet->obtain_header_rdlock(); | ||
| OLAPStatus acquire_reader_st = _tablet->capture_rs_readers(_params.version, &_params.rs_readers); | ||
| _tablet->release_header_lock(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should add a function in tablet who will encapsulate the lock and unlock operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently could not. because some times we do step 1 step 2 step 3 and step 3 is capture_rs_readers. If add lock in capture rs readers will be dead lock.
…hich cache is hit (apache#1400)
No description provided.