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

[IOTDB-774]Handle result of "show timeseries" as iterator #1417

Closed
wants to merge 18 commits into from
Closed

[IOTDB-774]Handle result of "show timeseries" as iterator #1417

wants to merge 18 commits into from

Conversation

SailVR
Copy link
Contributor

@SailVR SailVR commented Jun 24, 2020 •
edited
Loading

Currently, "show timeseries" will cache all timeseries in memory, which may cause OOM.

samperson1997
samperson1997 requested changes Jun 28, 2020
View reviewed changes
Copy link
Contributor

@samperson1997 samperson1997 left a comment

Choose a reason for hiding this comment

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

Please fix the potential bugs according to the failed tests in IoTDBSortedShowTimeseriesIT, IoTDBTagAlterIT and IoTDBTagIT

@SailVR
Copy link
Contributor Author

SailVR commented Jun 28, 2020

Please fix the potential bugs according to the failed tests in IoTDBSortedShowTimeseriesIT, IoTDBTagAlterIT and IoTDBTagIT

OK，Thanks

samperson1997
samperson1997 reviewed Jun 29, 2020
View reviewed changes
Copy link
Contributor

@samperson1997 samperson1997 left a comment

Choose a reason for hiding this comment

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

First I would recommend you to format the code style. For example, there should be a space before ,.
There are some duplicated methods in PlanExecutor, such as updateRecord. Please remove them after putting them in QueryUtils

server/src/main/java/org/apache/iotdb/db/service/TSServiceImpl.java Outdated Show resolved Hide resolved
samperson1997
samperson1997 approved these changes Jun 29, 2020
View reviewed changes
Copy link
Contributor

@samperson1997 samperson1997 left a comment

Choose a reason for hiding this comment

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

LGTM. I have rerun the failed Travis CI for you.

@SailVR SailVR closed this Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samperson1997 samperson1997 samperson1997 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SailVR @samperson1997