Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

Improve concurrency for needed parts. #3107

Merged
merged 14 commits into from
Mar 4, 2020

Conversation

thinker0
Copy link
Member

@thinker0 thinker0 commented Nov 9, 2018

Improve concurrency for needed parts.

  • Change concurrentHashMap
  • Improve thread safe OutputCollector
  • Improve thread safe IOutputCollector

#3104

@nwangtw
Copy link
Contributor

nwangtw commented Nov 9, 2018

That is the root cause of the exception? Nice find!

@thinker0
Copy link
Member Author

thinker0 commented Nov 9, 2018

@nwangtw

I created a Bolt with MoultiThread (finagle-http) async and ack all at once.

I think NPE is happening often.

heron-0.18.7 I can not create a maven package file, so I can not test it in my project.
Can you tell me how to package it so I can test it?

@nwangtw
Copy link
Contributor

nwangtw commented Nov 9, 2018

I see. Make sense then when you use multi threading. In this case you might be better to synchronized {} to make sure ack() is not called many times at the same time. OutputCollector has a queue in it and I am not sure the queue is thread-safe. In Heron engine, the core assumes single thread environment.

Internally, we build library and use it directly, instead of the ones in Maven.

bazel build --compilation_mode=dbg --config=darwin scripts/packages:tarpkgs

Copy link
Contributor

@nwangtw nwangtw left a comment

Choose a reason for hiding this comment

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

otherwise lgtm.

Copy link
Contributor

@nwangtw nwangtw left a comment

Choose a reason for hiding this comment

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

Please see above comments. I think these changes should be made before merging this PR to master. Thanks in advance.

@thinker0 thinker0 force-pushed the feature/concurent-metrics branch from 2198208 to 2e2702d Compare February 16, 2020 06:06
@thinker0 thinker0 changed the title Change concurrent Map Improve concurrency for needed parts. Feb 16, 2020
@thinker0 thinker0 requested a review from nwangtw February 16, 2020 06:45
@thinker0
Copy link
Member Author

Please review the PR. @nwangtw

@joshfischer1108
Copy link
Member

The changes after Ning's comments look fine to me. I'll leave it to @nwangtw to approve.

@thinker0
Copy link
Member Author

I see. Make sense then when you use multi threading. In this case you might be better to synchronized {} to make sure ack() is not called many times at the same time. OutputCollector has a queue in it and I am not sure the queue is thread-safe. In Heron engine, the core assumes single thread environment.

Internally, we build library and use it directly, instead of the ones in Maven.

bazel build --compilation_mode=dbg --config=darwin scripts/packages:tarpkgs

Made the OutputCollector thread-safe.

@nwangtw,

synchronized

works fine without it.
And it works well in Production.

@thinker0
Copy link
Member Author

thinker0 commented Mar 4, 2020

@joshfischer1108 PR Check ^^

@joshfischer1108 joshfischer1108 merged commit ee1c44a into apache:master Mar 4, 2020
@thinker0 thinker0 deleted the feature/concurent-metrics branch March 4, 2020 15:21
thinker0 added a commit to thinker0/heron that referenced this pull request Mar 17, 2020
* master:
  Improve concurrency for needed parts. (apache#3107)
  Add documents for setting up a docker based development environment (apache#3475)
  Patch to fix cppcheck with newer glibc (apache#3471)
  Make log/sink/consume Streamlet component support setName and setNumPartitions (apache#3459)
thinker0 added a commit to thinker0/heron that referenced this pull request Mar 21, 2020
* Change concurrent Map

* Change concurrent Map

* HashMap changes for unneeded parts.

* HashMap changes for unneeded parts.

* Review changes

* Changes HashMap for unneeded parts.

* Improve concurrency for needed parts.

* Remove unused imports.

* Remove unused imports.

* Remove unused imports.

* Fix NPE

(cherry picked from commit 545d381)

* Fix WhitespaceAround

* Add dummy Object

* Fix ConstantName

(cherry picked from commit 8d6d506)
thinker0 added a commit to thinker0/heron that referenced this pull request Mar 21, 2020
…hinker0/heron into feature/support-custom-metrics-rules-for-prometheus-sink

* 'feature/fix-prometheus-metrics' of https://github.com/thinker0/heron:
  Add Rules
  Update kafkaOffset metrics
  Improve concurrency for needed parts. (apache#3107)
  Add documents for setting up a docker based development environment (apache#3475)
  Patch to fix cppcheck with newer glibc (apache#3471)
  Make log/sink/consume Streamlet component support setName and setNumPartitions (apache#3459)
nwangtw added a commit that referenced this pull request Apr 2, 2020
* Support Java 11

* config travis to use oracle jdk 11

* Java 11 support (#3399)

* Support Java 11

* config travis to use oracle jdk 11

* Add check jdk version

* Fix command arguments.

Change insert gc_options

Update list

Fix gc-logging

* Add missing parameter

* typo

* Add pause time

* wip

* Support jmx_exporter format configuration.

* Fix checkstyle

* Remove unused

* Java 11 support (#3399)

* Support Java 11

* config travis to use oracle jdk 11

* Add check jdk version

* Fix command arguments.

Change insert gc_options

Update list

Fix gc-logging

* wip

* Support jmx_exporter format configuration.

* Fix checkstyle

* Remove unused

* Update kafkaOffset metrics

* Add Rules

* Make log/sink/consume Streamlet component support setName and setNumPartitions (#3459)

* Patch to fix cppcheck with newer glibc (#3471)

* Add documents for setting up a docker based development environment (#3475)

* Improve concurrency for needed parts. (#3107)

* Change concurrent Map

* Change concurrent Map

* HashMap changes for unneeded parts.

* HashMap changes for unneeded parts.

* Review changes

* Changes HashMap for unneeded parts.

* Improve concurrency for needed parts.

* Remove unused imports.

* Remove unused imports.

* Remove unused imports.

* Fix NPE

(cherry picked from commit 545d381)

* Fix WhitespaceAround

* Add dummy Object

* Fix ConstantName

(cherry picked from commit 8d6d506)

* Update kafkaOffset metrics

* Add Rules

* Update line is longer than 100 characters

* Update line is longer than 100 characters

* Add attrNameSnakeCase or other metrics fix

* fix checkstyle

Co-authored-by: Ning Wang <wangninggm@gmail.com>
Co-authored-by: Ning Wang <nwang@twitter.com>
Co-authored-by: Nicholas Nezis <nicholas.nezis@gmail.com>
sreev pushed a commit to sreev/incubator-heron that referenced this pull request Apr 9, 2020
* Change concurrent Map

* Change concurrent Map

* HashMap changes for unneeded parts.

* HashMap changes for unneeded parts.

* Review changes

* Changes HashMap for unneeded parts.

* Improve concurrency for needed parts.

* Remove unused imports.

* Remove unused imports.

* Remove unused imports.

* Fix NPE

(cherry picked from commit 545d381)

* Fix WhitespaceAround

* Add dummy Object

* Fix ConstantName

(cherry picked from commit 8d6d506)
sreev pushed a commit to sreev/incubator-heron that referenced this pull request Apr 9, 2020
* Support Java 11

* config travis to use oracle jdk 11

* Java 11 support (apache#3399)

* Support Java 11

* config travis to use oracle jdk 11

* Add check jdk version

* Fix command arguments.

Change insert gc_options

Update list

Fix gc-logging

* Add missing parameter

* typo

* Add pause time

* wip

* Support jmx_exporter format configuration.

* Fix checkstyle

* Remove unused

* Java 11 support (apache#3399)

* Support Java 11

* config travis to use oracle jdk 11

* Add check jdk version

* Fix command arguments.

Change insert gc_options

Update list

Fix gc-logging

* wip

* Support jmx_exporter format configuration.

* Fix checkstyle

* Remove unused

* Update kafkaOffset metrics

* Add Rules

* Make log/sink/consume Streamlet component support setName and setNumPartitions (apache#3459)

* Patch to fix cppcheck with newer glibc (apache#3471)

* Add documents for setting up a docker based development environment (apache#3475)

* Improve concurrency for needed parts. (apache#3107)

* Change concurrent Map

* Change concurrent Map

* HashMap changes for unneeded parts.

* HashMap changes for unneeded parts.

* Review changes

* Changes HashMap for unneeded parts.

* Improve concurrency for needed parts.

* Remove unused imports.

* Remove unused imports.

* Remove unused imports.

* Fix NPE

(cherry picked from commit 545d381)

* Fix WhitespaceAround

* Add dummy Object

* Fix ConstantName

(cherry picked from commit 8d6d506)

* Update kafkaOffset metrics

* Add Rules

* Update line is longer than 100 characters

* Update line is longer than 100 characters

* Add attrNameSnakeCase or other metrics fix

* fix checkstyle

Co-authored-by: Ning Wang <wangninggm@gmail.com>
Co-authored-by: Ning Wang <nwang@twitter.com>
Co-authored-by: Nicholas Nezis <nicholas.nezis@gmail.com>
nicknezis pushed a commit that referenced this pull request Sep 14, 2020
* Change concurrent Map

* Change concurrent Map

* HashMap changes for unneeded parts.

* HashMap changes for unneeded parts.

* Review changes

* Changes HashMap for unneeded parts.

* Improve concurrency for needed parts.

* Remove unused imports.

* Remove unused imports.

* Remove unused imports.

* Fix NPE

(cherry picked from commit 545d381)

* Fix WhitespaceAround

* Add dummy Object

* Fix ConstantName

(cherry picked from commit 8d6d506)
nicknezis added a commit that referenced this pull request Sep 14, 2020
* Support Java 11

* config travis to use oracle jdk 11

* Java 11 support (#3399)

* Support Java 11

* config travis to use oracle jdk 11

* Add check jdk version

* Fix command arguments.

Change insert gc_options

Update list

Fix gc-logging

* Add missing parameter

* typo

* Add pause time

* wip

* Support jmx_exporter format configuration.

* Fix checkstyle

* Remove unused

* Java 11 support (#3399)

* Support Java 11

* config travis to use oracle jdk 11

* Add check jdk version

* Fix command arguments.

Change insert gc_options

Update list

Fix gc-logging

* wip

* Support jmx_exporter format configuration.

* Fix checkstyle

* Remove unused

* Update kafkaOffset metrics

* Add Rules

* Make log/sink/consume Streamlet component support setName and setNumPartitions (#3459)

* Patch to fix cppcheck with newer glibc (#3471)

* Add documents for setting up a docker based development environment (#3475)

* Improve concurrency for needed parts. (#3107)

* Change concurrent Map

* Change concurrent Map

* HashMap changes for unneeded parts.

* HashMap changes for unneeded parts.

* Review changes

* Changes HashMap for unneeded parts.

* Improve concurrency for needed parts.

* Remove unused imports.

* Remove unused imports.

* Remove unused imports.

* Fix NPE

(cherry picked from commit 545d381)

* Fix WhitespaceAround

* Add dummy Object

* Fix ConstantName

(cherry picked from commit 8d6d506)

* Update kafkaOffset metrics

* Add Rules

* Update line is longer than 100 characters

* Update line is longer than 100 characters

* Add attrNameSnakeCase or other metrics fix

* fix checkstyle

Co-authored-by: Ning Wang <wangninggm@gmail.com>
Co-authored-by: Ning Wang <nwang@twitter.com>
Co-authored-by: Nicholas Nezis <nicholas.nezis@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants