You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/05/25 10:55:33 UTC

[GitHub] [druid] FrankChen021 opened a new pull request #11299: Fix permission problems in docker

FrankChen021 opened a new pull request #11299:
URL: https://github.com/apache/druid/pull/11299


   Fixes #11278 , #11298 
   
   ### Description
   
   To fix 11278, `mv` instruction is used to replace original `ln` instruction to eliminate symlink so that the docker image works with AWS Faragate. This change has been verified with the help of @mattmassicotte , reporter of that issue.
   
   To fix 11298, `/opt/data` is created in the Dockerfile so that a named volume could be used in the docker-compose. Since I don't make change bind volume in the example docker-compose to the named volume, I also add a note section in the document.
   
   Note: I'm still testing the changes on different platforms.
   
   This PR has:
   - [X] been self-reviewed.
      - [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
   - [ ] added documentation for new or modified features or behaviors.
   - [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
   - [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
   - [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
   - [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
   - [ ] added integration tests.
   - [] been tested in a test Druid cluster.
   


-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] FrankChen021 commented on pull request #11299: Fix permission problems in docker

Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on pull request #11299:
URL: https://github.com/apache/druid/pull/11299#issuecomment-848578103






-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] clintropolis commented on pull request #11299: Fix permission problems in docker

Posted by GitBox <gi...@apache.org>.
clintropolis commented on pull request #11299:
URL: https://github.com/apache/druid/pull/11299#issuecomment-849492783


   I think anything that makes the quick start work across platforms is agreeable; it is not very large data so disk size shouldn't matter afaik.


-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] FrankChen021 commented on pull request #11299: Fix permission problems in docker

Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on pull request #11299:
URL: https://github.com/apache/druid/pull/11299#issuecomment-850858074


   I tested these changes on 3 different host OS, all worked. See the description of this PR.


-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] FrankChen021 commented on pull request #11299: Fix permission problems in docker

Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on pull request #11299:
URL: https://github.com/apache/druid/pull/11299#issuecomment-849466170


   @clintropolis A named volume in docker-compose also shares contents among overlords, middle managers and historicals. At first I thought the reason why the host directory 'storge' is used has something to do with the disk size at first. Because by default, docker creates directory for a named volume under `/var` which usually has smaller disk size for druid data storage.  
   
   Since there's no special reasons to use a directory on the host OS to mount as a volume in docker, I think it's acceptable for us to use a named volume in the example docker-compose.yml for data storage, so that there won't be permission problems on Linux platforms. Another reason is that the docker-compose.yml is only an example, it's not recommended in production, the disk size problem of a named volume should not be a concern for us if we state it in the doc.
   
   What do you think ? @clintropolis @jihoonson @xvrl 
   If you agree, I will push a new commit.


-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] FrankChen021 removed a comment on pull request #11299: Fix permission problems in docker

Posted by GitBox <gi...@apache.org>.
FrankChen021 removed a comment on pull request #11299:
URL: https://github.com/apache/druid/pull/11299#issuecomment-848578189


   @jihoonson I don't know why host directory is used in the original docker-compose.yml, and I'm waiting for an answer from @clintropolis . If there's no special reason, I think it's acceptable to use a named volume in the example compose file. Before that, we have to execute a command to set the permission for linux systems.


-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] FrankChen021 commented on pull request #11299: Fix permission problems in docker

Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on pull request #11299:
URL: https://github.com/apache/druid/pull/11299#issuecomment-850857492


   > As I mentioned in the dev thread, it seems to me that the real problem is this docker image is not being tested properly. I'm not sure why the tests we have today don't catch this error. 
   
   I have the same question as you. I will check it when I'm free.
   


-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] FrankChen021 commented on pull request #11299: Fix permission problems in docker

Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on pull request #11299:
URL: https://github.com/apache/druid/pull/11299#issuecomment-850346582


   Thanks @xvrl , I have submitted a new commit to resolve all your comments. Test result will be updated in this weekend.


-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] jihoonson commented on pull request #11299: Fix permission problems in docker

Posted by GitBox <gi...@apache.org>.
jihoonson commented on pull request #11299:
URL: https://github.com/apache/druid/pull/11299#issuecomment-848573456


   @FrankChen021 thank you for making this PR. I tested your change on my machine, but, unfortunately, ingestion still fails with the same error. I'm using linux mint 20.1, which is based on Ubuntu Focal. Should we update the `docker-compose.yml` file per your suggestion in this PR instead of documenting it?


-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] clintropolis merged pull request #11299: Fix permission problems in docker

Posted by GitBox <gi...@apache.org>.
clintropolis merged pull request #11299:
URL: https://github.com/apache/druid/pull/11299


   


-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] xvrl commented on a change in pull request #11299: Fix permission problems in docker

Posted by GitBox <gi...@apache.org>.
xvrl commented on a change in pull request #11299:
URL: https://github.com/apache/druid/pull/11299#discussion_r641285685



##########
File path: distribution/docker/Dockerfile
##########
@@ -58,7 +58,10 @@ COPY --chown=druid:druid --from=builder /opt /opt
 COPY distribution/docker/druid.sh /druid.sh
 RUN mkdir /opt/druid/var \

Review comment:
       ```suggestion
   RUN mkdir /opt/druid/var /opt/data \
   ```




-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] xvrl commented on a change in pull request #11299: Fix permission problems in docker

Posted by GitBox <gi...@apache.org>.
xvrl commented on a change in pull request #11299:
URL: https://github.com/apache/druid/pull/11299#discussion_r641285805



##########
File path: distribution/docker/Dockerfile
##########
@@ -58,7 +58,10 @@ COPY --chown=druid:druid --from=builder /opt /opt
 COPY distribution/docker/druid.sh /druid.sh
 RUN mkdir /opt/druid/var \
  && chown druid:druid /opt/druid/var \
- && chmod 775 /opt/druid/var
+ && chmod 775 /opt/druid/var \

Review comment:
       ```suggestion
    && chmod 775 /opt/druid/var /opt/data \
   ```




-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] xvrl commented on a change in pull request #11299: Fix permission problems in docker

Posted by GitBox <gi...@apache.org>.
xvrl commented on a change in pull request #11299:
URL: https://github.com/apache/druid/pull/11299#discussion_r641285893



##########
File path: distribution/docker/Dockerfile
##########
@@ -58,7 +58,10 @@ COPY --chown=druid:druid --from=builder /opt /opt
 COPY distribution/docker/druid.sh /druid.sh
 RUN mkdir /opt/druid/var \
  && chown druid:druid /opt/druid/var \

Review comment:
       ```suggestion
    && chown druid:druid /opt/druid/var /opt/data \
   ```




-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] FrankChen021 commented on pull request #11299: Fix permission problems in docker

Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on pull request #11299:
URL: https://github.com/apache/druid/pull/11299#issuecomment-848585213


   @jihoonson will the ingestion success after executing the following command ?
   
   ```
   sudo docker exec --user root middlemanager chown druid:druid /opt/data
   ```


-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] xvrl commented on a change in pull request #11299: Fix permission problems in docker

Posted by GitBox <gi...@apache.org>.
xvrl commented on a change in pull request #11299:
URL: https://github.com/apache/druid/pull/11299#discussion_r641290198



##########
File path: distribution/docker/Dockerfile
##########
@@ -58,7 +58,10 @@ COPY --chown=druid:druid --from=builder /opt /opt
 COPY distribution/docker/druid.sh /druid.sh
 RUN mkdir /opt/druid/var \
  && chown druid:druid /opt/druid/var \
- && chmod 775 /opt/druid/var
+ && chmod 775 /opt/druid/var \
+ && mkdir /opt/data \

Review comment:
       can we add a comment to explain the difference between `/opt/data` and `/opt/druid/var` ? 




-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] clintropolis commented on pull request #11299: Fix permission problems in docker

Posted by GitBox <gi...@apache.org>.
clintropolis commented on pull request #11299:
URL: https://github.com/apache/druid/pull/11299#issuecomment-849377914


   >@jihoonson I don't know why host directory is used in the original docker-compose.yml, and I'm waiting for an answer from @clintropolis . If there's no special reason, I think it's acceptable to use a named volume in the example compose file. Before that, we have to execute a command to set the permission for linux systems.
   
   Sorry for the delay, as long as the same volume for storage is shared and available to the historical, middle manager, and overlord containers then everything should be ok. The important part is that it is shared between containers, because the storage directory is used as deep storage for the example docker cluster, so the segments and task logs that the peons running in the  middle manager publish can be then read by the overlord and historical. I don't think it matters too much however this happens.


-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] xvrl commented on a change in pull request #11299: Fix permission problems in docker

Posted by GitBox <gi...@apache.org>.
xvrl commented on a change in pull request #11299:
URL: https://github.com/apache/druid/pull/11299#discussion_r641291832



##########
File path: docs/tutorials/docker.md
##########
@@ -36,7 +36,11 @@ The Druid source code contains [an example `docker-compose.yml`](https://github.
 
 ### Compose file
 
-The example `docker-compose.yml` will create a container for each Druid service, as well as Zookeeper and a PostgreSQL container as the metadata store. Deep storage will be a local directory, by default configured as `./storage` relative to your `docker-compose.yml` file, and will be mounted as `/opt/data` and shared between Druid containers which require access to deep storage. The Druid containers are configured via an [environment file](https://github.com/apache/druid/blob/{{DRUIDVERSION}}/distribution/docker/environment).
+The example `docker-compose.yml` will create a container for each Druid service, as well as Zookeeper and a PostgreSQL container as the metadata store. 
+
+It will also create a named volumes `druid-data`, which is mounted as `opt/data` in container, as deep storage to keep and share segments and task logs among Druid services.

Review comment:
       I feel `druid-data` and `/opt/data` is a little generic and doesn't really convey the idea that this is meant for shared storage. `/opt/druid/var` is also used for local "data" so the distinction is not very clear.
   
   Maybe we should give it a more descriptive name such as `/opt/shared`, or something else if you have better suggestions, wdyt?




-- 
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: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org