Comments (5)
I have made some improvements to my fork that I think are going to simplify things:
The way I have things now:
- Most of the logic that was originally in
BinaryLogClient
moves toAbstractBinaryLogClient
. - Instead of invoking event and lifecycle listeners,
AbstractBinaryLogClient
just invokesprotected abstract
methods to allow subclasses to decide how to handle these. BinaryLogClient
subclassesAbstractBinaryLogClient
to implement the same behavior as before.- As of now, I've left my
BroadcastEventListener
andBroadcastLifecycleListener
refactoring intoBinaryLogClient
, but I'm not particularly attached to it now, and would be perfectly happy to restore this to how it was originally.
The idea is that it should be possible to write new subclasses of AbstractBinaryLogClient
that, by throwing an Exception, cause the client to abort and disconnect.
My next steps:
- Install vagrant, get the unit test suite running, and validate I haven't broken anything.
- Do a pull request
- Add tests to verify that the
AbstractBinaryLogClient
aborts gracefully if theonEvent
method fails; modify the code if this is not so. - Do a pull request
from mysql-binlog-connector-java.
Howdy,
Can I ask why didn't you go with registering (just an example) FailureIntolerantDelegatingEventListener which calls disconnect() on the client when some event listener (among registered) throws an exception, like so:
Note that implementation below is not thread-safe (for brevity).
public class FailureIntolerantDelegatingEventListener
implements BinaryLogClient.EventListener {
private final BinaryLogClient binaryLogClient;
private final List<EventListener> eventListeners =
new LinkedList<EventListener>();
public FailureIntolerantDelegatingEventListener(
BinaryLogClient binaryLogClient) {
this.binaryLogClient = binaryLogClient;
}
public void registerEventListener(EventListener eventListener) {
eventListeners.add(eventListener);
}
@Override
public void onEvent(Event event) {
for (EventListener eventListener : eventListeners) {
try {
eventListener.onEvent(event);
} catch (Exception e) {
binaryLogClient.disconnect();
break; // do not notify remaining event listeners
}
}
}
}
// and then somewhere in the code:
...
BinaryLogClient binaryLogClient = ...
FailureIntolerantDelegatingEventListener delegatingEventListener =
new FailureIntolerantDelegatingEventListener(binaryLogClient);
delegatingEventListener.registerEventListener(<event listener>);
delegatingEventListener.registerEventListener(<another event listener>);
...
binaryLogClient.registerEventListener(delegatingEventListener);
binaryLogClient.connect();
...
If, for some reason, you don't like the idea of EventListener being aware of BinaryLogClient and you really want BinaryLogClient itself to take care of any failures, then how about issue-15-proposed-change? It's basically few lines change which would allow to override ("in-place") notification strategy used by BinaryLogClient.
As for the "BinaryLogClient should only have one EventListener and one LifecycleListener", personally I'd rather not do that. Even if we set aside backward compatibility (as stable (API-wise) version is not in Maven Central yet) I would argue that it's too restrictive (=frustrating) (especially when you want to register (temporary) some tracing / monitoring listeners which do not care (or even know) about other registered ones (like in tests)). Moreover, no one is forcing to use binaryLogClient.registerEventListener more than once if you absolutely don't want to :).
Please let me know your thoughts on this.
from mysql-binlog-connector-java.
Can I ask why didn't you go with registering (just an example) FailureIntolerantDelegatingEventListener which calls disconnect() on the client when some event listener (among registered) throws an exception, like so [...]
A handful of reasons. First of all, well, I didn't think of it, for starters :-P.
Second: design-wise, I thought that BinaryLogClient
perhaps has too many responsibilities right now, so I sought to separate two of them (communication with the server vs. managing the listeners).
Third: implicitly swallowing the EventListener
s' exceptions strikes me as a biased, dangerous and complex default. By "biased," I mean that it's the behavior we want in some scenarios, like when you have disparate listeners sharing one client, and performing relatively safe actions in response to events (like invalidating in-memory caches). But in other cases, we really do want to fail fast.
For example, in my project I'm trying to record the history of changes to a table that is subject to frequent UPDATE statements. This means for each event, I have to:
- Insert a row into another database recording whether the event's row data, whether it was an insert, update or delete, and the timestamp of the event;
- Record the binlog filename and position into another table as the "high water mark";
- Take care to handle transactions correctly, so that rows and high water marks are only committed on XID events.
If the event listener for example fails to insert into the target, it really makes no sense to continue. Even worse, if a target insert failure is later followed by a success, then I've put the target into an erroneous state. So "swallow by default" strikes me as a dangerous default, in that it makes it easier for careless people to implement listeners like this one incorrectly. (Note that I count myself among the careless.)
It's also more complex to reason about, because just like I can write an EventListener
that disconnects the client on failure, somebody can write a LifecycleListener
that reconnects the client on a disconnect. So, arguably, your FailureIntolerantDelegatingEventListener
idea needs to be modified to unregister the listener in addition to disconnecting the client.
It's your code, however, so your call. I could certainly live with this proposal for now.
If, for some reason, you don't like the idea of EventListener being aware of BinaryLogClient and you really want BinaryLogClient itself to take care of any failures, [...]
I don't like it for this reason: such EventListener
s are harder to unit test. For example I wrote an automated test yesterday for my project's EventListener
by using the BinaryLogFileReader
to feed it some binlog files I generated by running some scripts against a fresh MySQL install.
Though in this case thankfully the FailureIntolerantDelegatingEventListener
delegates to a EventListener
that can be tested on its own. So again, I could live with that proposal.
[...] then how about issue-15-proposed-change? It's basically few lines change which would allow to override ("in-place") notification strategy used by BinaryLogClient.
To my eye, a subclass that overrode notifyEventListeners
to implement the behavior I propose would be violating its superclass' contract. For example, my subclass could simply not honor EventListener
registrations.
But again, your code, your call.
As for the "BinaryLogClient should only have one EventListener and one LifecycleListener", personally I'd rather not do that.
That was my first idea, but I changed my mind pretty quickly. The patch for #16 ended up not having this.
Summary: there are three proposals on the table that AFAIK can all be made to work:
- My pull request #16
- No code change; use a
FailureIntolerantDelegatingEventListener
or similar - c025f1e
from mysql-binlog-connector-java.
implicitly swallowing the EventListeners' exceptions strikes me as a biased, dangerous and complex default
You're probably right. I also tend to agree on "too many responsibilities" part. Let me think it through.
from mysql-binlog-connector-java.
Well, in the meantime, I've implemented a version of your FailureIntolerantDelegatingEventListener
in my project, but with one twist that I mentioned earlier—unsubscribe first, then disconnect. Looking good so far...
from mysql-binlog-connector-java.
Related Issues (20)
- NPE in TableMapEvent deserialization continuously HOT 1
- exception occured when i use function [JsonBinary.parseAsString]
- com.github.shyiko.mysql.binlog.network.AuthenticationException: Client does not support authentication protocol requested by server; consider upgrading MySQL client HOT 5
- ByteArrayInputStream has a performance problem HOT 2
- No break keyword in the switch-case code block.
- How to get only the binlog that listens to the specified db database HOT 1
- Is there any performance test report?
- BinaryLogClient auto detects wrongly MariaDB instead of MySQL HOT 1
- I want to get all the columns instead of filtering out the columns with a null value
- readMetadata HOT 1
- Provide ability to specify JDBC connection string HOT 1
- Failed to execute CommandLineRunner
- disconnect call getting blocked with keepalive thread
- [AWS Aurora] com.github.shyiko.mysql.binlog.network.AuthenticationException: unsupported authentication method: mysql_clear_password
- How to solve the binlog sync delay?
- The ROTATE event gets the wrong position
- Wrong value for time(6) data type
- Failed to deserialize data of EventHeaderV4
- [v0.25.3] Failed to deserialize data of EventHeaderV4. Possible fixes included.
- 字段类似是longtext,eventData.getRows()返回的是个地址
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from mysql-binlog-connector-java.