You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/09/02 18:47:15 UTC
[GitHub] [hadoop-ozone] nandakumar131 opened a new pull request #1378: HDDS-4133. Use new ContainerManager in SCM.
nandakumar131 opened a new pull request #1378:
URL: https://github.com/apache/hadoop-ozone/pull/1378
## What changes were proposed in this pull request?
Use new ContainerManager implementation in SCM
## What is the link to the Apache JIRA
HDDS-4133
## How was this patch tested?
Unit tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] GlenGeng commented on a change in pull request #1378: HDDS-4133. Use new ContainerManager in SCM.
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #1378:
URL: https://github.com/apache/hadoop-ozone/pull/1378#discussion_r488408833
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java
##########
@@ -242,19 +286,82 @@ public void removeContainerReplica(final ContainerID id,
@Override
public void updateDeleteTransactionId(
final Map<ContainerID, Long> deleteTransactionMap) throws IOException {
- throw new UnsupportedOperationException("Not yet implemented!");
+ lock.writeLock().lock();
+ try {
+ containerStateManager.updateDeleteTransactionId(deleteTransactionMap);
+ } finally {
+ lock.writeLock().unlock();
+ }
}
@Override
public ContainerInfo getMatchingContainer(final long size, final String owner,
- final Pipeline pipeline, final List<ContainerID> excludedContainerIDS) {
- throw new UnsupportedOperationException("Not yet implemented!");
+ final Pipeline pipeline, final Set<ContainerID> excludedContainerIDs) {
+ NavigableSet<ContainerID> containerIDs;
+ ContainerInfo containerInfo;
+ try {
+ synchronized (pipeline.getId()) {
Review comment:
why not synchronize on pipeline ?
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerID.java
##########
@@ -41,8 +41,8 @@
* @param id int
*/
private ContainerID(long id) {
- Preconditions.checkState(id > 0,
- "Container ID should be a positive. %s.", id);
+ Preconditions.checkState(id >= 0,
Review comment:
Is 0 a valid containerID ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] nandakumar131 commented on a change in pull request #1378: HDDS-4133. Use new ContainerManager in SCM.
Posted by GitBox <gi...@apache.org>.
nandakumar131 commented on a change in pull request #1378:
URL: https://github.com/apache/hadoop-ozone/pull/1378#discussion_r496151525
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java
##########
@@ -242,19 +286,82 @@ public void removeContainerReplica(final ContainerID id,
@Override
public void updateDeleteTransactionId(
final Map<ContainerID, Long> deleteTransactionMap) throws IOException {
- throw new UnsupportedOperationException("Not yet implemented!");
+ lock.writeLock().lock();
+ try {
+ containerStateManager.updateDeleteTransactionId(deleteTransactionMap);
+ } finally {
+ lock.writeLock().unlock();
+ }
}
@Override
public ContainerInfo getMatchingContainer(final long size, final String owner,
- final Pipeline pipeline, final List<ContainerID> excludedContainerIDS) {
- throw new UnsupportedOperationException("Not yet implemented!");
+ final Pipeline pipeline, final Set<ContainerID> excludedContainerIDs) {
+ NavigableSet<ContainerID> containerIDs;
+ ContainerInfo containerInfo;
+ try {
+ synchronized (pipeline.getId()) {
Review comment:
This is to avoid the warning that we get when we synchronize on the method parameter. This is just to fool the IDE.
In general, synchronizing on method parameter is bad. We have to fix this in a better way.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] nandakumar131 commented on a change in pull request #1378: HDDS-4133. Use new ContainerManager in SCM.
Posted by GitBox <gi...@apache.org>.
nandakumar131 commented on a change in pull request #1378:
URL: https://github.com/apache/hadoop-ozone/pull/1378#discussion_r496149425
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerID.java
##########
@@ -41,8 +41,8 @@
* @param id int
*/
private ContainerID(long id) {
- Preconditions.checkState(id > 0,
- "Container ID should be a positive. %s.", id);
+ Preconditions.checkState(id >= 0,
Review comment:
We don't create a container with ID 0, but 0 is a valid ID.
The reason for allowing 0 as a value for container ID is to avoid the explicit null check that we do in HDDS-1302.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org