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.

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

[IOTDB-1609] Print the actual size of plan when it exceeds the wal_buffer_size #4279

Merged
merged 4 commits into from Nov 2, 2021

Conversation

Cpaulyz
Copy link
Contributor

@Cpaulyz Cpaulyz commented Oct 30, 2021

Contributions:

  • Print the actual size of plan when it exceeds the wal_buffer_size
    • Use PhysicalPlan.serialize(DataOutputStream stream) to get actual size of plan after serialization.
    • Print infomation in IOException message
  • Add some test in testOverSizedWAL to verify the actual size of plan
    • If wal_buffer_size is less than actual size, it would throw IOException.
    • If wal_buffer_size is more than or equal to actual size, it would not throw IOException.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 66.983% when pulling 6093d10 on Cpaulyz:fix/wal_buffer_size into b5ef4d8 on apache:master.

Comment on lines 116 to 119
DataOutputStream dos = new DataOutputStream(new ByteArrayOutputStream());
plan.serialize(dos);
int neededSize = dos.size();
dos.close();
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that there is no need to really do the serialization to get the actual size, maybe you can add a new method like getSerializedSize() in PhysicalPlan, each actual plan class to override that method to calculate its own size. In this way, you can avoid allocate extra memory in this BufferOverflowException situation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that there is no need to really do the serialization to get the actual size, maybe you can add a new method like getSerializedSize() in PhysicalPlan, each actual plan class to override that method to calculate its own size. In this way, you can avoid allocate extra memory in this BufferOverflowException situation.

Yes, I totally agree with what you said. In fact, I wanted to do the same thing at first.

However, I noticed that there are quite a lot of classes extends PhysicalPlan. It needs to modify code massively. Considering that getSerializedSize() is only used for this exception and exception may be occurred rarely, I allocate extra memory to get the actual size.

Do you think it's necessary to do what you said? If so, I may need more time to implement getSerializedSize() in each actual plan class.

@JackieTien97 JackieTien97 merged commit 25a98ee into apache:master Nov 2, 2021
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.

None yet

3 participants