You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/03/09 23:50:25 UTC

[GitHub] [hbase] markrmiller opened a new pull request #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…

markrmiller opened a new pull request #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…
URL: https://github.com/apache/hbase/pull/1261
 
 
   …eforeInitHandler.
   
   id2Call is a HashMap and has a call back method that accesses it that can be run via an executor as well as another method accessing it that can be run from a different thread.
   id2Call should likely be a ConcurrentHashMap to be shared like this.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase commented on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…
URL: https://github.com/apache/hbase/pull/1261#issuecomment-596843144
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 20s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --findbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   7m 27s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 32s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 26s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 32s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   6m 56s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 34s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 34s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 15s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 28s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m  5s |  hbase-client in the patch passed.  |
   |  |   |  32m 46s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.7 Server=19.03.7 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1261/1/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1261 |
   | JIRA Issue | HBASE-23952 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 05db727839ae 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 31484f007f |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1261/1/testReport/ |
   | Max. process+thread count | 376 (vs. ulimit of 10000) |
   | modules | C: hbase-client U: hbase-client |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1261/1/console |
   | versions | git=2.17.1 maven=2018-06-17T18:33:14Z) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [hbase] Apache9 commented on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…

Posted by GitBox <gi...@apache.org>.
Apache9 commented on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…
URL: https://github.com/apache/hbase/pull/1261#issuecomment-596846820
 
 
   In netty a handler will only be called in one thread, and we only access the id2Call map in the handler specific methods, so I do not think we need to change it to `ConcurrentHashMap`.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [hbase] saintstack commented on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…

Posted by GitBox <gi...@apache.org>.
saintstack commented on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…
URL: https://github.com/apache/hbase/pull/1261#issuecomment-602776184
 
 
   Progress here? Single-threaded or concurrent access? Should I resolve the issue? Thanks.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [hbase] Apache9 commented on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…

Posted by GitBox <gi...@apache.org>.
Apache9 commented on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…
URL: https://github.com/apache/hbase/pull/1261#issuecomment-596876547
 
 
   EventExecutor is a single thread executor...
   
   EventExecutorGroup is what you expected, a thread pool...
   
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [hbase] markrmiller edited a comment on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…

Posted by GitBox <gi...@apache.org>.
markrmiller edited a comment on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…
URL: https://github.com/apache/hbase/pull/1261#issuecomment-596869131
 
 
   I think the issue is that Netty can use different threads to access this data structure, ie:
   
   org.apache.hbase.thirdparty.io.netty.channel.AbstractChannelHandlerContext
   ```
   static void invokeUserEventTriggered(final AbstractChannelHandlerContext next, final Object event) {
           ObjectUtil.checkNotNull(event, "event");
           EventExecutor executor = next.executor();
           if (executor.inEventLoop()) {
               next.invokeUserEventTriggered(event);
           } else {
               executor.execute(new Runnable() {
                   @Override
                   public void run() {
                       next.invokeUserEventTriggered(event);
                   }
               });
           }
       }
   ```
   
   
   If more than one thread is going to access a shared field, you have to publish it properly - either with memory barriers or it has to be final or effectively final. It seems very difficult to ensure that this is thread safe or will remain thread safe over time.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase commented on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…
URL: https://github.com/apache/hbase/pull/1261#issuecomment-596841920
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 31s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --findbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   6m 33s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 32s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m  4s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 28s |  hbase-client in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   6m 15s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 30s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 30s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   4m 53s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 27s |  hbase-client in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m  0s |  hbase-client in the patch passed.  |
   |  |   |  27m 24s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.7 Server=19.03.7 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1261/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1261 |
   | JIRA Issue | HBASE-23952 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 827816f88e7e 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 31484f007f |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1261/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-client.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1261/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-client.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1261/1/testReport/ |
   | Max. process+thread count | 442 (vs. ulimit of 10000) |
   | modules | C: hbase-client U: hbase-client |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1261/1/console |
   | versions | git=2.17.1 maven=2018-06-17T18:33:14Z) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [hbase] Apache9 commented on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…

Posted by GitBox <gi...@apache.org>.
Apache9 commented on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…
URL: https://github.com/apache/hbase/pull/1261#issuecomment-602985842
 
 
   As from my knowledge it is single threaded, otherwise there will be bunch of code need to be fixed, as we use netty a lot in our code...

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [hbase] markrmiller commented on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…

Posted by GitBox <gi...@apache.org>.
markrmiller commented on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…
URL: https://github.com/apache/hbase/pull/1261#issuecomment-596869131
 
 
   I think the issue is that Netty can use different threads to access this data structure, ie:
   
   `    
   static void invokeUserEventTriggered(final AbstractChannelHandlerContext next, final Object event) {
           ObjectUtil.checkNotNull(event, "event");
           EventExecutor executor = next.executor();
           if (executor.inEventLoop()) {
               next.invokeUserEventTriggered(event);
           } else {
               executor.execute(new Runnable() {
                   @Override
                   public void run() {
                       next.invokeUserEventTriggered(event);
                   }
               });
           }
       }
   `
   
   If more than one thread is going to access a shared field, you have to publish it properly - either with memory barriers or it has to be final or effectively final. It seems very difficult to ensure that this is thread safe or will remain thread safe over time.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase commented on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…
URL: https://github.com/apache/hbase/pull/1261#issuecomment-596844099
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 31s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m 34s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 30s |  master passed  |
   | +0 :ok: |  spotbugs  |   1m  3s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   1m  0s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m  3s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 29s |  hbase-client: The patch generated 0 new + 0 unchanged - 3 fixed = 0 total (was 3)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  16m  7s |  Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.  |
   | +1 :green_heart: |  findbugs  |   1m  7s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 14s |  The patch does not generate ASF License warnings.  |
   |  |   |  37m 21s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.7 Server=19.03.7 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1261/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1261 |
   | JIRA Issue | HBASE-23952 |
   | Optional Tests | dupname asflicense spotbugs findbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 8fe714216573 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 31484f007f |
   | Max. process+thread count | 93 (vs. ulimit of 10000) |
   | modules | C: hbase-client U: hbase-client |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1261/1/console |
   | versions | git=2.17.1 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [hbase] markrmiller edited a comment on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…

Posted by GitBox <gi...@apache.org>.
markrmiller edited a comment on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…
URL: https://github.com/apache/hbase/pull/1261#issuecomment-596869131
 
 
   I think the issue is that Netty can use different threads to access this data structure, ie:
   
   ```
   static void invokeUserEventTriggered(final AbstractChannelHandlerContext next, final Object event) {
           ObjectUtil.checkNotNull(event, "event");
           EventExecutor executor = next.executor();
           if (executor.inEventLoop()) {
               next.invokeUserEventTriggered(event);
           } else {
               executor.execute(new Runnable() {
                   @Override
                   public void run() {
                       next.invokeUserEventTriggered(event);
                   }
               });
           }
       }
   ```
   
   
   If more than one thread is going to access a shared field, you have to publish it properly - either with memory barriers or it has to be final or effectively final. It seems very difficult to ensure that this is thread safe or will remain thread safe over time.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [hbase] saintstack edited a comment on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…

Posted by GitBox <gi...@apache.org>.
saintstack edited a comment on issue #1261: HBASE-23952: Address thread safety issue with Map used in BufferCallB…
URL: https://github.com/apache/hbase/pull/1261#issuecomment-602776184
 
 
   Progress here? Single-threaded or concurrent access? Should I close the issue? Thanks.

----------------------------------------------------------------
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


With regards,
Apache Git Services