[jira] Updated: (ZOOKEEPER-59) Synchronized block in NIOServerCnxn
[
https://issues.apache.org/jira/browse/ZOOKEEPER-59?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Flavio Paiva Junqueira updated ZOOKEEPER-59:
Status: Patch Available (was: Open)
Synchronized block in NIOServerCnxn
---
Key: ZOOKEEPER-59
URL: https://issues.apache.org/jira/browse/ZOOKEEPER-59
Project: Zookeeper
Issue Type: Bug
Components: server
Reporter: Flavio Paiva Junqueira
Assignee: Flavio Paiva Junqueira
Fix For: 3.3.0
Attachments: ZOOKEEPER-59.patch, ZOOKEEPER-59.patch
There are two synchronized blocks locking on different objects, and to me
they should be guarded by the same object. Here are the parts of the code I'm
talking about:
{noformat}
nioservercnxn.readrequ...@444
...
synchronized (this) {
outstandingRequests++;
// check throttling
if (zk.getInProcess() factory.outstandingLimit) {
disableRecv();
// following lines should not be needed since we are
already
// reading
// } else {
// enableRecv();
}
}
{noformat}
{noformat}
nioservercnxn.sendrespo...@740
...
synchronized (this.factory) {
outstandingRequests--;
// check throttling
if (zk.getInProcess() factory.outstandingLimit
|| outstandingRequests 1) {
sk.selector().wakeup();
enableRecv();
}
}
{noformat}
I think the second one is correct, and the first synchronized block should be
guarded by this.factory.
This could be related to issue ZOOKEEPER-57, but I have no concrete
indication that this is the case so far.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-59) Synchronized block in NIOServerCnxn
[
https://issues.apache.org/jira/browse/ZOOKEEPER-59?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Flavio Paiva Junqueira updated ZOOKEEPER-59:
Attachment: ZOOKEEPER-59.patch
I have updated the patch in the following way:
* I have changed synchronization blocks to reflect the discussion of this
issue. In the trunk code, there is at least one instance increment
outstandingRequests that is not synchronized with this. I have also tried to
make sure that all necessary blocks of code that update the SelectionKey are
synchronized with factory.
* I have not included the change of ZooKeeperServer that corresponds to making
requestsInProcess an AtomicInteger. This is really a minor change, and not
strictly necessary.
Ben made this comment that the methods of SelectionKey are thread-safe.
However, I believe the synchronization blocks are there to make the block of
code atomic, and not because of the individual calls. As mentioned above, I
have tried to make sure that all blocks are properly synchronized. I'm not
exactly sure, though, why some of the blocks related to SelectionKey operations
are synchronized. I have the impression that they have been introduced
conservatively, but I would appreciate if someone with an opinion could shed
some light here.
Synchronized block in NIOServerCnxn
---
Key: ZOOKEEPER-59
URL: https://issues.apache.org/jira/browse/ZOOKEEPER-59
Project: Zookeeper
Issue Type: Bug
Components: server
Reporter: Flavio Paiva Junqueira
Assignee: Flavio Paiva Junqueira
Fix For: 3.3.0
Attachments: ZOOKEEPER-59.patch, ZOOKEEPER-59.patch
There are two synchronized blocks locking on different objects, and to me
they should be guarded by the same object. Here are the parts of the code I'm
talking about:
{noformat}
nioservercnxn.readrequ...@444
...
synchronized (this) {
outstandingRequests++;
// check throttling
if (zk.getInProcess() factory.outstandingLimit) {
disableRecv();
// following lines should not be needed since we are
already
// reading
// } else {
// enableRecv();
}
}
{noformat}
{noformat}
nioservercnxn.sendrespo...@740
...
synchronized (this.factory) {
outstandingRequests--;
// check throttling
if (zk.getInProcess() factory.outstandingLimit
|| outstandingRequests 1) {
sk.selector().wakeup();
enableRecv();
}
}
{noformat}
I think the second one is correct, and the first synchronized block should be
guarded by this.factory.
This could be related to issue ZOOKEEPER-57, but I have no concrete
indication that this is the case so far.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-462) Last hint for open ledger
[ https://issues.apache.org/jira/browse/ZOOKEEPER-462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Flavio Paiva Junqueira updated ZOOKEEPER-462: - Fix Version/s: (was: 3.3.0) 3.4.0 This issue needs more discussion, and the feature it proposes is not strictly necessary, so I recommend that we move it to 3.4.0. Last hint for open ledger - Key: ZOOKEEPER-462 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-462 Project: Zookeeper Issue Type: New Feature Components: contrib-bookkeeper Reporter: Flavio Paiva Junqueira Assignee: Flavio Paiva Junqueira Fix For: 3.4.0 Attachments: ZOOKEEPER-462.patch In some use cases of BookKeeper, it is useful to be able to read from a ledger before closing the ledger. To enable such a feature, the writer has to be able to communicate to a reader how many entries it has been able to write successfully. The main idea of this jira is to continuously update a znode with the number of successful writes, and a reader can, for example, watch the node for changes. I was thinking of having a configuration parameter to state how often a writer should update the hint on ZooKeeper (e.g., every 1000 requests, every 10,000 requests). Clearly updating more often increases the overhead of writing to ZooKeeper, although the impact on the performance of writes to BookKeeper should be minimal given that we make an asynchronous call to update the hint. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-683) LogFormatter fails to parse transactional log files
[ https://issues.apache.org/jira/browse/ZOOKEEPER-683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12840417#action_12840417 ] Henry Robinson commented on ZOOKEEPER-683: -- Patch looks good to me - +1. LogFormatter fails to parse transactional log files --- Key: ZOOKEEPER-683 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-683 Project: Zookeeper Issue Type: Bug Components: server Affects Versions: 3.2.2 Reporter: Patrick Hunt Assignee: Patrick Hunt Priority: Blocker Fix For: 3.3.0 Attachments: ZOOKEEPER-683.patch LogFormatter fails to parse txn log files - seems the tool was never updated to handle FileHeader. It would be good to update the docs on txn log file to include detail on the file format. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-59) Synchronized block in NIOServerCnxn
[
https://issues.apache.org/jira/browse/ZOOKEEPER-59?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12840464#action_12840464
]
Hadoop QA commented on ZOOKEEPER-59:
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12437656/ZOOKEEPER-59.patch
against trunk revision 915956.
+1 @author. The patch does not contain any @author tags.
-1 tests included. The patch doesn't appear to include any new or modified
tests.
Please justify why no tests are needed for this patch.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac
compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs warnings.
+1 release audit. The applied patch does not increase the total number of
release audit warnings.
-1 core tests. The patch failed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results:
http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/125/testReport/
Findbugs warnings:
http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/125/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output:
http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/125/console
This message is automatically generated.
Synchronized block in NIOServerCnxn
---
Key: ZOOKEEPER-59
URL: https://issues.apache.org/jira/browse/ZOOKEEPER-59
Project: Zookeeper
Issue Type: Bug
Components: server
Reporter: Flavio Paiva Junqueira
Assignee: Flavio Paiva Junqueira
Fix For: 3.3.0
Attachments: ZOOKEEPER-59.patch, ZOOKEEPER-59.patch
There are two synchronized blocks locking on different objects, and to me
they should be guarded by the same object. Here are the parts of the code I'm
talking about:
{noformat}
nioservercnxn.readrequ...@444
...
synchronized (this) {
outstandingRequests++;
// check throttling
if (zk.getInProcess() factory.outstandingLimit) {
disableRecv();
// following lines should not be needed since we are
already
// reading
// } else {
// enableRecv();
}
}
{noformat}
{noformat}
nioservercnxn.sendrespo...@740
...
synchronized (this.factory) {
outstandingRequests--;
// check throttling
if (zk.getInProcess() factory.outstandingLimit
|| outstandingRequests 1) {
sk.selector().wakeup();
enableRecv();
}
}
{noformat}
I think the second one is correct, and the first synchronized block should be
guarded by this.factory.
This could be related to issue ZOOKEEPER-57, but I have no concrete
indication that this is the case so far.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
