Improve p2p bind and db lock err msgs #503

aionick · 2018-05-23T19:49:15Z

Description

Please include a brief summary of the change that this pull request proposes. Include any relevant motivation and context. List any dependencies required for this change.

This is two PRs merged into one... most of this is separating all of the inner classes in P2pMgr out into their own files and minor refactorings associated with that. There is also some improvement to the p2p error message when unable to bind to a port and the db error messages for when 2 instances try to use the same db.

Fixes Issue #

Type of change

Insert x into the following checkboxes to confirm (eg. [x]):

Bug fix.
New feature.
Enhancement.
Breaking change (a fix or feature that causes existing functionality to not work as expected).
Requires documentation update.

Testing

Please describe the tests you used to validate this pull request. Provide any relevant details for test configurations as well as any instructions to reproduce these results.

Used manual testing, running two kernels on the same port and then running two kernels on the same db instance for each db we support. For the refactoring bit: ran kernel with miners and checked against all existing tests cases.

Verification

Insert x into the following checkboxes to confirm (eg. [x]):

I have self-reviewed my own code and conformed to the style guidelines of this project.
New and existing tests pass locally with my changes.
I have added tests for my fix or feature.
I have made appropriate changes to the corresponding documentation.
My code generates no new warnings.
Any dependent changes have been made.

aion-sridevi · 2018-05-23T20:35:54Z

modDbImpl/src/org/aion/db/impl/h2/H2MVMap.java

@@ -148,7 +148,8 @@ public boolean open() {
        } catch (Exception e) {
            if (e instanceof NullPointerException) {
                LOG.error("Failed to open the database " + this.toString()
-                        + ". A probable cause is that the H2 database cannot access the file path.", e);
+                    + ". A probable cause is that the H2 database cannot access the file path. "
+                    + "Check that you do not have two instances running on the same database.", e);


Minor: May be change line 152 to "Check if you have two instances running" instead of "Check that you do not" and everywhere with the same statement.

Merge branch 'fix_separate_tasks' of https://github.com/aionick/aion into fix_err_msgs

aionick · 2018-05-25T13:22:07Z

I updated this PR because its dependent PR changed, commit 5f4ecec is the only one in here to pay attention to. Only 1 file in this PR is dependent, that is P2pMgr, and the change that I've made here is in lines 213-214, a catch SocketException block.

Sorry for the confusion

AionJayT · 2018-05-28T01:49:31Z

modP2pImpl/src/org/aion/p2p/impl1/P2pMgr.java

@@ -992,79 +155,76 @@ public void run() {
            tcpServer.socket().bind(new InetSocketAddress(Node.ipBytesToStr(selfIp), selfPort));
            tcpServer.register(selector, SelectionKey.OP_ACCEPT);

-            Thread thrdIn = new Thread(new TaskInbound(), "p2p-in");
+            Thread thrdIn = new Thread(getInboundInstance(), "p2p-in");
+            //            Thread thrdIn = new Thread(new TaskInbound(), "p2p-in");


can remove this line

AionJayT · 2018-05-29T14:23:38Z

related to #69

AionJayT · 2018-05-30T13:43:27Z

modAionImpl/test/org/aion/zero/impl/sync/BlockPropagationTest.java

+        public boolean isSyncSeedsOnly() { throw new IllegalStateException("not implemented."); }
+
+        @Override
+        public int getTxBroadCastRoute() { throw new IllegalStateException("not implemented."); }


this method should remove, also the interface class.

AlexandraRoatis

please make sure the license for the files is correct
(optional) it would be nice to pass a logger object to all the tasks and log the messages instead of using System.out; check TaskImportBlocks.java for reference
(optional) throughout the code I've seen many one liner if statements; I would prefer surrounding the then clause with { and }

AlexandraRoatis · 2018-05-31T20:34:31Z

modAionImpl/test/org/aion/zero/impl/sync/BlockPropagationTest.java

@@ -2,6 +2,7 @@

 import static com.google.common.truth.Truth.assertThat;

+import java.io.IOException;


this import is not used

AlexandraRoatis · 2018-05-31T20:43:15Z

modP2pImpl/src/org/aion/p2p/impl/comm/Node.java

@@ -31,6 +31,7 @@
 import java.util.Arrays;
 import java.util.regex.Pattern;
 import org.aion.p2p.INode;
+import org.aion.p2p.IPeerMetric;

 /*


should be /**

AlexandraRoatis · 2018-05-31T20:44:53Z

modP2p/src/org/aion/p2p/IPeerMetric.java

@@ -0,0 +1,15 @@
+package org.aion.p2p;
+
+public interface IPeerMetric {


would be nice to have some java doc for this new interface

AlexandraRoatis · 2018-05-31T21:29:05Z

modP2pImpl/src/org/aion/p2p/impl1/tasks/TaskInbound.java

+     * @throws IOException IOException
+     */
+    private int readHeader(final ChannelBuffer _cb, ByteBuffer readBuffer, int cnt)
+            throws IOException {


throws IOException may be unnecessary

AlexandraRoatis · 2018-05-31T21:29:27Z

modP2pImpl/src/org/aion/p2p/impl1/tasks/TaskInbound.java

+     * @throws IOException IOException
+     */
+    private int readBody(final ChannelBuffer _cb, ByteBuffer readBuffer, int cnt)
+            throws IOException {


throws IOException may be unnecessary

aionick · 2018-06-01T13:47:25Z

addressed the comments, working on the optionals now

aionick added 13 commits May 18, 2018 13:31

first refactor: separate TaskInbound

6fd52f2

second refactor: separate TaskSend

dc0cc75

third refactor: separate TaskReceive

4cf4642

fourth refactor: separate TaskStatus

91829ec

fifth refactor: separate TaskConnectPeers

0b981ec

sixth refactor: separate TaskClear

44e73a0

moved MsgOut/In classes, renames and cleanups

4d8faec

moved accept,read others into TaskInbound, cleanups

d1aba20

added licenses, more cleanups

e7f2386

refactoring to improve readability

a6b558a

moved tasks to separate package

7182785

add null check back in

bfd5c85

improved binding and db locking error messages

5f4ecec

AionJayT added this to the v0.2.8 - University Peak milestone May 23, 2018

AionJayT requested review from qoire, AlexandraRoatis, AionJayT and aion-sridevi May 23, 2018 19:54

AionJayT added enhancement New feature or request bug Something isn't working labels May 23, 2018

aion-sridevi reviewed May 23, 2018

View reviewed changes

aionick added 4 commits May 24, 2018 08:41

changed to check if you have..

8522525

separated MsgIn and MsgOut

f31e6ba

defined fields in terms of interfaces

1d48630

keep this PR up to date with its dependent PR.

89a28b4

Merge branch 'fix_separate_tasks' of https://github.com/aionick/aion into fix_err_msgs

aionick closed this May 25, 2018

AionJayT reopened this May 25, 2018

AionJayT requested changes May 28, 2018

View reviewed changes

AionJayT approved these changes May 28, 2018

View reviewed changes

removed commented-out code

9b0e045

AionJayT requested changes May 30, 2018

View reviewed changes

made txBroadCastRoute field visible

325f0a7

AlexandraRoatis suggested changes May 31, 2018

View reviewed changes

AionJayT approved these changes Jun 1, 2018

View reviewed changes

updated licenses and other minor fixes

9c65721

added braces to one-line ifs

8def32b

AlexandraRoatis approved these changes Jun 1, 2018

View reviewed changes

aion-sridevi approved these changes Jun 1, 2018

View reviewed changes

AionJayT merged commit 1d44cbd into aionnetwork:master-pre-merge Jun 1, 2018

AionJayT mentioned this pull request Jun 14, 2018

merge v0.2.8 features into master #525

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve p2p bind and db lock err msgs #503

Improve p2p bind and db lock err msgs #503

aionick commented May 23, 2018 •

edited

Loading

aion-sridevi May 23, 2018

aionick commented May 25, 2018 •

edited

Loading

AionJayT May 28, 2018

AionJayT commented May 29, 2018

AionJayT May 30, 2018

AlexandraRoatis left a comment •

edited

Loading

AlexandraRoatis May 31, 2018

AlexandraRoatis May 31, 2018

AlexandraRoatis May 31, 2018

AlexandraRoatis May 31, 2018

AlexandraRoatis May 31, 2018

aionick commented Jun 1, 2018

		@@ -2,6 +2,7 @@

		import static com.google.common.truth.Truth.assertThat;

		import java.io.IOException;

		@@ -0,0 +1,15 @@
		package org.aion.p2p;

		public interface IPeerMetric {

Improve p2p bind and db lock err msgs #503

Improve p2p bind and db lock err msgs #503

Conversation

aionick commented May 23, 2018 • edited Loading

Description

Type of change

Testing

Verification

Choose a reason for hiding this comment

aionick commented May 25, 2018 • edited Loading

Choose a reason for hiding this comment

AionJayT commented May 29, 2018

Choose a reason for hiding this comment

AlexandraRoatis left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aionick commented Jun 1, 2018

aionick commented May 23, 2018 •

edited

Loading

aionick commented May 25, 2018 •

edited

Loading

AlexandraRoatis left a comment •

edited

Loading