Skip to content

Conversation

@umamaheswararao
Copy link
Contributor

What changes were proposed in this pull request?

Added allocateBlock API in MockOmTransport.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-5350

How was this patch tested?

Added a test which triggers allocateBlock. Without source changes, test fails and with source changes test passed.

@umamaheswararao umamaheswararao requested review from elek and sodonnel June 16, 2021 14:46
return OzoneManagerProtocolProtos.AllocateBlockResponse.newBuilder()
.setKeyLocation(
blockAllocator.allocateBlock(allocateBlockRequest.getKeyArgs())
.iterator().next()).build();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me add loop here for all KeyArgs available

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at SinglePipelineBlockAllocator, I am not sure it can return multiple results, or that you can request multiple blocks in one go from the AllocateBlockRequest. However it does return an Iterable<>, so you have to call next on it as you have done.

The response AllocateBlockResponse object just contains a single "KeyLocation", which contains a single "BlockID" and "Pipeline". Therefore I think your code is correct and it does not need a loop here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh yeah. Looks like your are right. API name says keyLocation, but not keyLocations.

Copy link
Member

Choose a reason for hiding this comment

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

It's possible to return with multiple key locations if the keyArgs contains a size which is huge enough. Today it's not supported by the block allocated, but looks to be safer to add all the results to the setKeyLocation...

Copy link
Member

Choose a reason for hiding this comment

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

Oh, thanks for the offline discussion with you, I got it. Pure allocateBlock supports one keyLocation. Open key supports multiple location. Strange, but this is what we have. Thanks to help me learninig it.

Copy link
Contributor

@sodonnel sodonnel left a comment

Choose a reason for hiding this comment

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

This change LGTM.

@umamaheswararao umamaheswararao merged commit b563669 into apache:master Jun 23, 2021
errose28 added a commit to errose28/ozone that referenced this pull request Jul 7, 2021
* master: (28 commits)
  HDDS-5332. Add a new column family and a service provider in Recon DB for Namespace Summaries (apache#2366)
  HDDS-5405. Refactor pom files for HadoopRpc and Grpc/Ratis compilation properties. (apache#2386)
  HDDS-5406. add proto version to all the proto files. (apache#2385)
  HDDS-5398. Avoid object creation in ReplicationManger debug log statements (apache#2379)
  HDDS-5396. Fix negligence issue conditional expressions in MockCRLStore.java (apache#2380)
  HDDS-5395. Avoid unnecessary numKeyOps.incr() call in OMMetrics (apache#2374)
  HDDS-5389. Include ozoneserviceid in fs.defaultFS when configuring o3fs (apache#2370)
  HDDS-5383. Eliminate expensive string creation in debug log messages (apache#2372)
  HDDS-5380. Get more accurate space info for DedicatedDiskSpaceUsage. (apache#2365)
  HDDS-5341. Container report processing is single threaded (apache#2338)
  HDDS-5387. ProfileServlet to move the default output location to an ozone specific directory (apache#2368)
  HDDS-5289. Update container's deleteTransactionId on creation of the transaction in SCM. (apache#2361)
  HDDS-5369. Cleanup unused configuration related to SCM HA (apache#2359)
  HDDS-5381. SCM terminated with exit status 1: null. (apache#2362)
  HDDS-5353. Avoid unnecessary executeBatch call in insertAudits (apache#2342)
  HDDS-5350 :  Add allocate block support in MockOmTransport (apache#2341). Contributed by Uma Maheswara Rao G.
  HDDS-4926. Support start/stop for container balancer via command line (apache#2278)
  HDDS-5269. Datandoe with low ratis log volume space should not be considered for new pipeline allocation. (apache#2344)
  HDDS-5367. Update modification time when updating quota/storageType/versioning (apache#2355)
  HDDS-5352. java.lang.ClassNotFoundException: org/eclipse/jetty/alpn/ALPN (apache#2347)
  ...
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.

4 participants