Page MenuHomePhabricator
Differential D4014

[services] Remove `container_name` from docker-compose file
ClosedPublic
Actions

Authored by • max on May 12 2022, 4:59 AM.
Tags
None
Referenced Files
F5084945: D4014.diff
Sat, Mar 29, 3:22 PM
Unknown Object (File)
Mon, Mar 17, 5:50 PM
Unknown Object (File)
Thu, Mar 13, 12:37 PM
Unknown Object (File)
Thu, Mar 13, 12:37 PM
Unknown Object (File)
Thu, Mar 13, 12:37 PM
Unknown Object (File)
Thu, Mar 13, 12:37 PM
Unknown Object (File)
Thu, Mar 13, 12:30 PM
Unknown Object (File)
Feb 27 2025, 11:54 AM
View All Files
Subscribers
• abosh
• adrian
ashoat
atul
• karol
tomek

Details

Reviewers
• jim
tomek
• karol
ashoat
Commits
rCOMM53b692443067: [services] Remove `container_name` from docker-compose file
Summary

Because Docker container names must be unique, we cannot scale a service beyond 1 container that's why we need to remove container_name from
service's docker-compose file.

Related diff: D3847

Linear task: ENG-1129

Test Plan

Run docker compose up tunnelbroker-server, docker compose up backup-server, docker compose up blob-server.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

• max created this revision.May 12 2022, 4:59 AM
Herald added subscribers: • abosh, • karol, atul and 3 others. · View Herald TranscriptMay 12 2022, 4:59 AM
• max edited the summary of this revision. (Show Details)May 12 2022, 5:00 AM
• max added reviewers: • jim, tomek, • karol.
Harbormaster completed remote builds in B9033: Diff 12576.May 12 2022, 5:04 AM
• max requested review of this revision.May 12 2022, 5:04 AM
• max added a child revision: D3964: [services] Tunnelbroker - RabbitMQ Docker image for dev environment.May 12 2022, 5:06 AM
• max mentioned this in D3964: [services] Tunnelbroker - RabbitMQ Docker image for dev environment.May 12 2022, 5:10 AM
ashoat accepted this revision.May 12 2022, 12:42 PM

This looks simple so I'll just accept

tomek accepted this revision.May 13 2022, 1:52 AM
This revision is now accepted and ready to land.May 13 2022, 1:52 AM
• max updated this revision to Diff 12650.May 13 2022, 5:04 AM

Rebase on master.

Harbormaster completed remote builds in B9091: Diff 12650.May 13 2022, 5:10 AM
Closed by commit rCOMM53b692443067: [services] Remove `container_name` from docker-compose file (authored by • max). · Explain WhyMay 13 2022, 5:27 AM
This revision was automatically updated to reflect the committed changes.
• max added a commit: rCOMM53b692443067: [services] Remove `container_name` from docker-compose file.

Revision Contents
Changeset List

PathSize
services/
5 lines

Diff 12651

View Options

services/docker-compose.yml

#include "DatabaseManagerBase.h" #include "DatabaseManagerBase.h"
#include "Item.h" #include "Item.h"
#include <aws/core/utils/Outcome.h> #include <aws/core/utils/Outcome.h>
#include <aws/dynamodb/model/BatchWriteItemRequest.h>
#include <aws/dynamodb/model/BatchWriteItemResult.h>
#include <aws/dynamodb/model/DeleteItemRequest.h> #include <aws/dynamodb/model/DeleteItemRequest.h>
#include <iostream> #include <iostream>
namespace comm { namespace comm {
namespace network { namespace network {
namespace database { namespace database {
Show All 23 Linesvoid DatabaseManagerBase::innerRemoveItem(const Item &item) {
const Aws::DynamoDB::Model::DeleteItemOutcome &outcome = const Aws::DynamoDB::Model::DeleteItemOutcome &outcome =
getDynamoDBClient()->DeleteItem(request); getDynamoDBClient()->DeleteItem(request);
if (!outcome.IsSuccess()) { if (!outcome.IsSuccess()) {
throw std::runtime_error(outcome.GetError().GetMessage()); throw std::runtime_error(outcome.GetError().GetMessage());
} }
} }
void DatabaseManagerBase::innerBatchWriteItem(
const std::string &tableName,
std::vector<Aws::DynamoDB::Model::WriteRequest> &writeRequests) {
// Split write requests to chunks by MAX_DYNAMODB_BATCH_ITEMS size and write
// them by batch
Aws::DynamoDB::Model::BatchWriteItemOutcome outcome;
std::vector<Aws::DynamoDB::Model::WriteRequest> writeRequestsChunk;
std::vector<Aws::DynamoDB::Model::WriteRequest>::iterator chunkPositionStart,
chunkPositionEnd;
for (size_t i = 0; i < writeRequests.size(); i += MAX_DYNAMODB_BATCH_ITEMS) {
chunkPositionStart = writeRequests.begin() + i;
chunkPositionEnd = writeRequests.begin() +
std::min(writeRequests.size(), i + MAX_DYNAMODB_BATCH_ITEMS);
writeRequestsChunk = std::vector<Aws::DynamoDB::Model::WriteRequest>(
chunkPositionStart, chunkPositionEnd);
tomekUnsubmitted
Done
Inline Actions
Aws::DynamoDB::Model::BatchWriteItemOutcome outcome;
- std::vector<Aws::DynamoDB::Model::WriteRequest> writeRequestsChunk;
- std::vector<Aws::DynamoDB::Model::WriteRequest>::iterator chunkPositionStart,
- chunkPositionEnd;
for (size_t i = 0; i < writeRequests.size(); i += MAX_DYNAMODB_BATCH_ITEMS) {
- chunkPositionStart = writeRequests.begin() + i;
- chunkPositionEnd = writeRequests.begin() +
+ auto chunkPositionStart = writeRequests.begin() + i;
+ auto chunkPositionEnd = writeRequests.begin() +
std::min(writeRequests.size(), i + MAX_DYNAMODB_BATCH_ITEMS);
- writeRequestsChunk = std::vector<Aws::DynamoDB::Model::WriteRequest>(
+ auto writeRequestsChunk = std::vector<Aws::DynamoDB::Model::WriteRequest>(
chunkPositionStart, chunkPositionEnd);
Aws::DynamoDB::Model::BatchWriteItemRequest writeBatchRequest;

You can simplify the code by avoiding reusing the variables and using auto

tomek: You can simplify the code by avoiding reusing the variables and using `auto`
maxAuthorUnsubmitted
Done
Inline Actions

You can simplify the code by avoiding reusing the variables and using auto

I'm not a big fan of using auto where the type can be declared explicitly. Also, reusing the same variables is a part of loop optimization here (we don't create new ones inside the loop and reuse existing allocated memory for it).

max: > You can simplify the code by avoiding reusing the variables and using `auto` I'm not a big…
tomekUnsubmitted
Not Done
Inline Actions

Also, reusing the same variables is a part of loop optimization here

Have you measured the performance difference? I don't think this type of premature optimization is beneficial - if the variable is used only locally, it is easier to reason about it: you don't have to think about its initial value and about possible usages after the loop. Also, having a variable defined inside a loop might enable some optimization (I'm guessing now). Currently, we only reuse the name - new variables are constructed in every iteration.

So overall, we have a lot more important performance issues and saving a couple of machine instructions wouldn't be noticeable. On the other hand, from readability point of view, this is a noticeable difference - you are certain that the variable is used only where it is valid.

tomek: > Also, reusing the same variables is a part of loop optimization here Have you measured the…
Aws::DynamoDB::Model::BatchWriteItemRequest writeBatchRequest;
writeBatchRequest.AddRequestItems(tableName, writeRequestsChunk);
outcome = getDynamoDBClient()->BatchWriteItem(writeBatchRequest);
if (!outcome.IsSuccess()) {
throw std::runtime_error(outcome.GetError().GetMessage());
}
tomekUnsubmitted
Done
Inline Actions
writeBatchRequest.AddRequestItems(tableName, writeRequestsChunk);
- outcome = getDynamoDBClient()->BatchWriteItem(writeBatchRequest);
+ auto outcome = getDynamoDBClient()->BatchWriteItem(writeBatchRequest);
if (!outcome.IsSuccess()) {
throw std::runtime_error(outcome.GetError().GetMessage());
}
}
}

outcome isn't used outside of this loop so we can declare it here

tomek: `outcome` isn't used outside of this loop so we can declare it here
maxAuthorUnsubmitted
Done
Inline Actions

outcome isn't used outside of this loop so we can declare it here

Same as above. I'm sure we should not use auto, where the type can explicitly be used, and allocating and reusing this variable here, is an optimized way from my perspective.

max: > `outcome` isn't used outside of this loop so we can declare it here Same as above. I'm sure…
}
}
} // namespace database } // namespace database
} // namespace network } // namespace network
} // namespace comm } // namespace comm