Add comments to AllocateBatch in pkg/ddc/base/portallocator/bitmap_allocator.go#5800
Conversation
…locator.go Signed-off-by: Caterpillar <w@caterpillar.ink>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @caterpillar-1. Thanks for your PR. I'm waiting for a fluid-cloudnative member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds documentation for the AllocateBatch method in the BitMapAllocator. The review feedback identifies inaccuracies in the documentation, specifically regarding the claim that ports are allocated consecutively and the description of the error return value, as the function terminates the process on failure rather than returning an error.
| } | ||
|
|
||
| // AllocateBatch allocates a batch of ports from the bitmap allocator. | ||
| // This function attempts to allocate the specified number of consecutive ports and returns them as a slice. |
There was a problem hiding this comment.
The comment states that the function attempts to allocate "consecutive" ports. However, the implementation calls AllocateNext() in a loop, which retrieves available ports from the bitmap but does not guarantee they are contiguous.
| // This function attempts to allocate the specified number of consecutive ports and returns them as a slice. | |
| // This function attempts to allocate the specified number of ports and returns them as a slice. |
| // | ||
| // Returns: | ||
| // - ports ([]int): A slice containing the allocated port numbers | ||
| // - err (error): Error if allocation fails (note: function exits process on failure) |
There was a problem hiding this comment.
The documentation for the err return value is misleading. Since the function terminates the process on failure (as noted in the implementation and the function description), it never actually returns a non-nil error to the caller. If the function returns, err will always be nil.
| // - err (error): Error if allocation fails (note: function exits process on failure) | |
| // - err (error): Always nil if the function returns (note: function exits process on failure) |
Signed-off-by: Caterpillar <w@caterpillar.ink>
caterpillar-1
force-pushed
the
docs/comments_bitmap_allocator_AllocateBatch
branch
from
48a409f to
4c6d463
Compare
|
1 participant
Ⅰ. Describe what this PR does
Add comments to AllocateBatch in pkg/ddc/base/portallocator/bitmap_allocator.go
Ⅱ. Does this pull request fix one issue?
fixes #5799
III. Special notes for reviews