You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/06/05 13:13:23 UTC
[GitHub] [pulsar] shink opened a new pull request, #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
shink opened a new pull request, #15939:
URL: https://github.com/apache/pulsar/pull/15939
Master Issue: #8843
### Motivation
- Enable spotbugs for `pulsar-io-mongo`
### Modifications
- Add `spotbugs-maven-plugin` to `pulsar-io-mongo`
### Verifying this change
```shell
$ mvn clean install -pl pulsar-io/mongo
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on a diff in pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on code in PR #15939:
URL: https://github.com/apache/pulsar/pull/15939#discussion_r891873686
##########
pulsar-io/mongo/src/main/java/org/apache/pulsar/io/mongodb/MongoSink.java:
##########
@@ -68,7 +68,7 @@ public class MongoSink implements Sink<byte[]> {
private MongoCollection<Document> collection;
- private List<Record<byte[]>> incomingList;
+ private List<Record<byte[]>> incomingList = Lists.newArrayList();
Review Comment:
```suggestion
private List<Record<byte[]>> incomingList = new ArrayList<>();
```
It's recommended to use `ArrayList` instead of `Lists.newArrayList()` in guava annotation.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15939:
URL: https://github.com/apache/pulsar/pull/15939#issuecomment-1207316221
The pr had no activity for 30 days, mark with Stale label.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] shink commented on a diff in pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
Posted by GitBox <gi...@apache.org>.
shink commented on code in PR #15939:
URL: https://github.com/apache/pulsar/pull/15939#discussion_r889833519
##########
pulsar-io/mongo/src/main/java/org/apache/pulsar/io/mongodb/MongoSource.java:
##########
@@ -133,9 +134,12 @@ public void onNext(ChangeStreamDocument<Document> doc) {
recordValue.put("ns", doc.getNamespace());
recordValue.put("operation", doc.getOperationType());
- consume(new DocRecord(
- Optional.of(doc.getDocumentKey().toJson()),
- mapper.writeValueAsString(recordValue).getBytes(StandardCharsets.UTF_8)));
+ BsonDocument key = doc.getDocumentKey();
+ if (key != null) {
+ consume(new DocRecord(
+ Optional.of(key.toJson()),
+ mapper.writeValueAsString(recordValue).getBytes(StandardCharsets.UTF_8)));
+ }
Review Comment:
Or in this way?
```java
BsonDocument key = doc.getDocumentKey();
consume(new DocRecord(
key != null ? Optional.of(key.toJson()) : Optional.empty(),
mapper.writeValueAsString(recordValue).getBytes(StandardCharsets.UTF_8)));
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] shink commented on pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
Posted by GitBox <gi...@apache.org>.
shink commented on PR #15939:
URL: https://github.com/apache/pulsar/pull/15939#issuecomment-1147057015
SpotBugs found an error:
```
Error: Medium: Inconsistent synchronization of org.apache.pulsar.io.mongodb.MongoSink.incomingList; locked 83% of time [org.apache.pulsar.io.mongodb.MongoSink, org.apache.pulsar.io.mongodb.MongoSink, org.apache.pulsar.io.mongodb.MongoSink, org.apache.pulsar.io.mongodb.MongoSink, org.apache.pulsar.io.mongodb.MongoSink, org.apache.pulsar.io.mongodb.MongoSink] Unsynchronized access at MongoSink.java:[line 101]Synchronized access at MongoSink.java:[line 118]Synchronized access at MongoSink.java:[line 119]Synchronized access at MongoSink.java:[line 132]Synchronized access at MongoSink.java:[line 136]Synchronized access at MongoSink.java:[line 137] IS2_INCONSISTENT_SYNC
```
So, is the `incomingList` in [these codes](https://github.com/apache/pulsar/blob/02dca31a78523a7d061ac1d6702ff6600a7f4ec4/pulsar-io/mongo/src/main/java/org/apache/pulsar/io/mongodb/MongoSink.java#L107-L159) not synchronized?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] shink commented on pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
Posted by GitBox <gi...@apache.org>.
shink commented on PR #15939:
URL: https://github.com/apache/pulsar/pull/15939#issuecomment-1148217622
>
@Shoothzj Can you help me on this problem? I don't know why the code is not synchronized. But the following code is correct when I test it locally. Change the object
```java
private final Object lock = new Object();
```
```java
synchronized (lock) {
incomingList.add(record);
currentSize = incomingList.size();
}
```
```java
synchronized (lock) {
if (incomingList.isEmpty()) {
return;
}
recordsToInsert = incomingList;
incomingList = Lists.newArrayList();
}
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on a diff in pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on code in PR #15939:
URL: https://github.com/apache/pulsar/pull/15939#discussion_r889843297
##########
pulsar-io/mongo/src/main/java/org/apache/pulsar/io/mongodb/MongoSource.java:
##########
@@ -133,9 +134,12 @@ public void onNext(ChangeStreamDocument<Document> doc) {
recordValue.put("ns", doc.getNamespace());
recordValue.put("operation", doc.getOperationType());
- consume(new DocRecord(
- Optional.of(doc.getDocumentKey().toJson()),
- mapper.writeValueAsString(recordValue).getBytes(StandardCharsets.UTF_8)));
+ BsonDocument key = doc.getDocumentKey();
+ if (key != null) {
+ consume(new DocRecord(
+ Optional.of(key.toJson()),
+ mapper.writeValueAsString(recordValue).getBytes(StandardCharsets.UTF_8)));
+ }
Review Comment:
@shink I think we don't need to throw exception. We also can
```java
Optional.ofNullable(doc.getDocumentKey()).map(BsonDocument::toJson),
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] shink commented on a diff in pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
Posted by GitBox <gi...@apache.org>.
shink commented on code in PR #15939:
URL: https://github.com/apache/pulsar/pull/15939#discussion_r889937127
##########
pulsar-io/mongo/src/main/java/org/apache/pulsar/io/mongodb/MongoSource.java:
##########
@@ -133,9 +134,12 @@ public void onNext(ChangeStreamDocument<Document> doc) {
recordValue.put("ns", doc.getNamespace());
recordValue.put("operation", doc.getOperationType());
- consume(new DocRecord(
- Optional.of(doc.getDocumentKey().toJson()),
- mapper.writeValueAsString(recordValue).getBytes(StandardCharsets.UTF_8)));
+ BsonDocument key = doc.getDocumentKey();
+ if (key != null) {
+ consume(new DocRecord(
+ Optional.of(key.toJson()),
+ mapper.writeValueAsString(recordValue).getBytes(StandardCharsets.UTF_8)));
+ }
Review Comment:
@Shoothzj Good idea. Done.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] shink commented on a diff in pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
Posted by GitBox <gi...@apache.org>.
shink commented on code in PR #15939:
URL: https://github.com/apache/pulsar/pull/15939#discussion_r891875715
##########
pulsar-io/mongo/src/main/java/org/apache/pulsar/io/mongodb/MongoSink.java:
##########
@@ -68,7 +68,7 @@ public class MongoSink implements Sink<byte[]> {
private MongoCollection<Document> collection;
- private List<Record<byte[]>> incomingList;
+ private List<Record<byte[]>> incomingList = Lists.newArrayList();
Review Comment:
@Shoothzj Done.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15939:
URL: https://github.com/apache/pulsar/pull/15939#issuecomment-1146804050
We need each findbugsExclude.xml each module? I not sure if it's a good way. cc @codelipenghui @eolivelli
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun closed pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
Posted by GitBox <gi...@apache.org>.
tisonkun closed pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
URL: https://github.com/apache/pulsar/pull/15939
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15939:
URL: https://github.com/apache/pulsar/pull/15939#issuecomment-1146802530
@shink:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? 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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] shink commented on a diff in pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
Posted by GitBox <gi...@apache.org>.
shink commented on code in PR #15939:
URL: https://github.com/apache/pulsar/pull/15939#discussion_r889832030
##########
pulsar-io/mongo/src/main/java/org/apache/pulsar/io/mongodb/MongoSource.java:
##########
@@ -133,9 +134,12 @@ public void onNext(ChangeStreamDocument<Document> doc) {
recordValue.put("ns", doc.getNamespace());
recordValue.put("operation", doc.getOperationType());
- consume(new DocRecord(
- Optional.of(doc.getDocumentKey().toJson()),
- mapper.writeValueAsString(recordValue).getBytes(StandardCharsets.UTF_8)));
+ BsonDocument key = doc.getDocumentKey();
+ if (key != null) {
+ consume(new DocRecord(
+ Optional.of(key.toJson()),
+ mapper.writeValueAsString(recordValue).getBytes(StandardCharsets.UTF_8)));
+ }
Review Comment:
Should an exception be thrown if `doc.getDocumentKey()` returns null?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15939:
URL: https://github.com/apache/pulsar/pull/15939#issuecomment-1147073555
@shink I think define plugin in each module is ok.(We may can't fix every module spotbugs in one PR) Before I was wondering there is no need to create so many `findbugsExclude.xml`, After a dive to project, looking at the current state of the project, let's do this first(each module a `findbugsExclude.xml`)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] shink commented on pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
Posted by GitBox <gi...@apache.org>.
shink commented on PR #15939:
URL: https://github.com/apache/pulsar/pull/15939#issuecomment-1146966972
@Shoothzj How about adding the `spotbugs-maven-plugin` to the parent project and having all child projects inherit.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] shink commented on pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
Posted by GitBox <gi...@apache.org>.
shink commented on PR #15939:
URL: https://github.com/apache/pulsar/pull/15939#issuecomment-1149401410
/pulsarbot run-failure-checks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on pull request #15939: [SpotBugs][pulsar-io] Enable spotbugs for pulsar-io-mongo
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #15939:
URL: https://github.com/apache/pulsar/pull/15939#issuecomment-1340342233
Closed as stale and conflict. Please rebase and submit a new patch if it's still relevant.
I'd prefer to enable spotbugs in the root module.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org