On 10/29/2013 11:47 PM, xor wrote:
> [re-arranged part of the quotes for better structure of the reply, I
> don't want to answer to the same question in three different areas]
> 
> On Monday, October 28, 2013 12:29:51 AM Steve Dougherty wrote:
[...]
>> WoT is synchronizing entire categories of stuff? Always sending 
>> everything is insanity. I really think WoT should allow much more 
>> selectiveness. The concept of keeping state up to date with an
>> initial synchronization and subsequent notifications is great, but
>> this returning too much information in a way that requires a lot
>> of additional processing to be useful. Requiring such processing
>> invites another wave of duplicate implementations of WoT-wrangling
>> code in clients.
[...]
> It is NOT only for debugging purposes, and it is not insane. Forget
> about WOT for a moment and think of a database as a set of objects,
> entities, files, things, whatever. If you have a set of things, and
> you want to keep synchronized copies of that set among different
> machines, you CAN keep it up-to-date by only sending single elements
> of the set as they change -  BUT you can only do that if you knew 
> what the initial state was, i.e. the FULL initial set. This is not an
>  implementation detail. Its logically impossible to build a full view
> of something if you ONLY get notified about changes. You HAVE TO know
> the initial state for the change-only notifications to be able to
> provide you with a full view of everything. Lets look at this from a
> different example which you have worked in length with: Version
> control, for example Git. If you update a git repository by "git
> pull", you WILL only download changes, the "diffs" of each commit.
> But that only works if you did a "git clone" before. And what does
> git clone do? It copies the WHOLE repository. Would it be possible to
> compile the code WITHOUT a clone, just by having the diffs? No way.
> Only having random pieces of modifications of a bunch of source code
> produces random garbage which is not equal to the full source code.

I understand the concept of sending initial state and keeping it up to
date with incremental changes. I think it is a very good idea. My
objection is that clients of event-notifications without filters will
have to write duplicate filter implementations to make use of the data.
This is similar to clients writing duplicate event-driven wrappers. We
agree that filters are good, and I think WoT should implement them
itself before event-notifications are declared usable. To do otherwise
is to place undue burden on client application developers. If there is
something that almost every client application will do with the data it
gets, I think WoT should be able to do it for them.

>> I will go further and say I think it very rarely makes sense for a 
>> client application to have to pull in (and keep up-to-date with!)
>> WoT's entire state in the first place. I would expect queries and
>> subscribing to changes in their results.
> 
> Forum systems such as Freetalk *do* want to give every identity which
> is trusted a chance to post messages, so they need to know about
> them. That will apply to most identities in the database. So
> synchronizing almost the full set of identities will happen.

Freetalk doesn't need to be kept up to date on every identity though. It
needs only identities with a score >= 0 and a Freetalk context.

[...]
>>>> This message is send via the synchronous FCP-API: You can
>>>> signal that processing it failed by returning an error in the
>>>> FCP message processor. This allows your client to be programmed
>>>> in a transactional style: If part of the transaction which 
>>>> stores the dataset fails, you can just roll it back and signal
>>>> the error to WOT. It will rollback the subscription then and 
>>>> send an "Error" message, indicating that subscribing failed.
>>>> You must file another subscription attempt then.
>> 
>> This sounds like it's just for debugging purposes. Is there a way
>> in which failure to apply an incremental change to the state is not
>> a bug?
> 
> No, its NOT just for debugging. Transactional programming means that
> you assume that the contents of your database are always 100% valid
> in terms of logically making sense. You are free to do partial
> changes to the database which cause a non-senseful state for as long
> as after the transaction the database is 100% semantically correct
> again. Disruptive changes are always wrapped in a transaction, and
> the transaction is either commited fully or thrown away completely. 
> So transactions are atomic. Leaving out a single part of a
> event-notifications subscription chain means that the semantic
> integrity of the client would be damaged.:The data does not match the
> actual WOT database contents anymore if you leave out the initial 
> syncronization or an event in the event-chain!
> 
> The good thing about transactions it that they can guard against
> failure of LOTS of things in a complex process. If a minor very low
> level detail of low level code fails, you just throw out an exception
> to the high level code. It will rollback() the transaction, and
> event-notifications will cause it to happen again. While progamming
> transactionally, you basically assume that any code can fail, even if
> you cannot come up with an example of WHY it might fail. . The core
> point I'm trying to make is that the "top level" code which actually
>  initiates the transactions is what makes that assumption. It
> considers the actual workers which do the processing of the
> transaction as little magic black boxes which can fail for weird
> reasons, and are free to do so. It might be out of memory, or
> whatever. You throw one worker away, and let the next try to do the
> transaction again. The top level code in our case is the event- 
> notifications server of WOT: It allows event-processors to fail, and
> can resend the event then. A simple example of something which can
> and will happen in practice: Shutdown. At shutdown, there might be
> threads running to process event-notifications. Some of them might be
> in "waiting" state, waiting for a lock on the database where they
> store identities/trusts/scores. The database might be terminated in 
> between, so the waiting event-processors cannot process the event. As
> event- notifications might be persistent in the future, it is
> necessary that WOT resents the aborted event-notification as the
> client plugin is started again.

Okay, so it's to allow resuming persistent subscriptions. Does a
persistent subscription mean skipping sending the initial state? My
initial reaction is to be fearful of strange bugs with such a setup. I'm
not sure that having such a feature strikes me as high priority.

>>>> The second message is formatted as: "Message" = "Subscribed" 
>>>> "SubscriptionID" = Random {@link UUID} of the Subscription. 
>>>> "To" = Same as the "To" field of your original message.
>> 
>> I'm curious why this ID is not in the first message? Why two
>> messages?
> 
> If you indicate failure of processing the intial synchronization
> message, you are not subscribed because you don't have the full
> dataset available and it is impossible to obtain it without the
> initial synchronization.

I don't see the client indicating success processing the initial state
before the ID is sent.

[...]
>> Would it be appropriate to have an error code so that some error
>> types can be machine-readable? I remember running into difficulties
>> with this sort of thing with my Infocalypse patches.
> 
> The "Description" is the actual class name of the exception, so it
> should be safe to assume that the same literal value is always used.

Ah, I missed that field. That makes sense.

[...]
>>>> These messages are also send with the synchronous FCP API. In
>>>> opposite to the initial synchronization message, by replying
>>>> with failure to the synchronous FCP call, you can signal that
>>>> you want to receive the same notification again.
>> This is inconsistent,
> 
> It is, and that annoyed my while implementing it. I would rather have
> both the synchronization and the event-notifications be automatically
> resent. But automatic resending of the initial synchronization would
> be difficult to implement. Also, it is a LARGE dataset and therefore
> we should avoid sending it too often.

Fair enough.

>> and it is confusing to me. If there is not an error during
>> transmission what could resending a message achieve? The client
>> already got the message.
> 
> As explained above, the goal is to allow transactional programming in
> the client. A transactional, and as such in this case especially
> database-based client ONLY keeps its transactional database as
> storage: One of the point of using a database is to be able to store
> data which is so large that it doesn't fit into memory. Therefore, a
> truly database-based client doesn't keep ANYTHING in memory
> permanently. If an event causes the need for it do to something, it 
> only queries from the database what is needed to process the event -
> which will fit into memory. Afterwards processing finishes, all in
> memory objects are flushed. For example consider forum systems such
> as Freetalk: If the user wants to display the threads in a forum, you
> only query the first 50 threads in the forum and display them as page
> 1. You do NOT load the other 1000000 pages into memory, they wouldn't
> fit and the user isn't viewieng them now anyway.
> 
> So if the transaction aimed at storing it to the database fails,
> there is no other place where the client can store it to retry the
> transaction, because conceptually it shall not keep ANYTHING in
> memory permanently. If it did keep a queue of failed
> event-notifications in memory, what happens if 100000 of them fail in
> a row? Out of memory! What happens if they had initially failed due
> to out of memory? Out of memory ^ 2. The source of information which
> triggered the transaction must re-trigger it if it failed because it
> has stored its existence anyway. In our case the source is WOT.

So it is for fault-tolerance in the case of errors not related to the
content of the messages themselves? A failure in the client to apply a
changeset seems like something it might deal with itself by
unsubscribing and resubscribing, if anything. Involving WoT in that
process does not look like an immediate benefit to me, though I can see
it being a refinement.

>>>> After a typical delay of {@link 
>>>> SubscriptionManager#PROCESS_NOTIFICATIONS_DELAY}, it will be
>>>> re-sent. There is a maximal amount of {@link 
>>>> SubscriptionManager#DISCONNECT_CLIENT_AFTER_FAILURE_COUNT}
>>>> failures per FCP- Client. If you exceed this limit, your
>>>> subscriptions will be terminated. You will receive an
>>>> "Unsubscribed" message then as long as your client has not 
>>>> terminated the FCP connection. See {@link 
>>>> #handleUnsubscribe(SimpleFieldSet)}. The fact that you can
>>>> request a notification to be re-sent may also be used to
>>>> program your client in a transactional style. If the
>>>> transaction which processes an event-notification fails, you
>>>> can indicate failure to the synchronous FCP sender and WOT will
>>>> then re-send the notification, causing the transaction to be 
>>>> retried.
>> The notion of resending enabling transactional state changes seems
>> at odds with being unsubscribed after hitting a limit on them.
> 
> This is necessary to guard against bugs which cause WOT not noticing
> that a client has disconnected. If the client has disconnected, its
> notifications' resinding will fail continously and WOT will just
> disconnect it due to that then. If there was no such mechanism, ghost
> clients would cause the database to grow indefinitely from
> notifications which cannot be deployed.

Relieving WoT of the need to keep messages around makes sense.
Is there no distinction between a failure to send a message to a
disconnected client and a client returning a "failure to process: please
resend" message?

>>>> If your client is shutting down or not interested in the
>>>> subscription anymore, you should send an "Unsubscribe"
>>>> message. See {@link #handleUnsubscribe(SimpleFieldSet)}. This
>>>> will make sure that WOT stops gathering data for your
>>>> subscription, which would be expensive to do if its not even
>>>> needed. But if you cannot send the message anymore due to a
>>>> dropped connection, the subscription will be terminated
>>>> automatically after some time due to notification-deployment
>>>> failing. Nevertheless, please always unsubscribe when
>>>> possible.
>> 
>> Does WoT not have visibility on when FCP connections close? How
>> does WoT handle pushing messages over FCP? I had to jump through
>> hoops to do that with Infocalypse.
> 
> What it keeps as a "connection" for being able to send to the client
> in a pushing manner is WeakReference<PluginReplySender> .
> PluginReplySender is the object which you get when a plugin uses a
> PluginTalker to send a message to you as a server. In other words:
> The client uses PluginTalker to send the original message to the
> server. The server's message handler gets called by the node, and the
> node gives it a PluginReplySender for being able to answer. By
> keeping a WeakReference<PluginReplySender>, WOT can keep the sender
> in memory for being able to deploy notifications in the future.
> Because it is a WeakReference, it will get garbage-collected if the
> client drops its PluginTalker. WOT also montiors a ReferenceQueue on
> the WeakReference objects, which allows it to notice if one of their
> pointers got GC'ed, and purge the WeakReference object itself. 
> However, this is not immediate, and it is not a strong mechanism - GC
> might happen very far in the future.
> 
> So to answer the original question: The PluginTalker API just does
> not support explicit disconnection. I decided it would be easier to
> just deal with it as is than changing it. Further, network
> connections can randomly drop dead, so you need to assume that both
> graceful disconnection as well as random death (= timeout) can 
> happen. The "Unsubscribe" message is the graceful disconnection. The
> exceeding of the event-notification failure counter deals with random
> death. Graceful disconnection is usually implemented in addition to
> timeout for performance reasons: The quicker we get told that the
> client is disconnected, the quicker we can stop gathering data for
> it. So that's why you are also able to unsubscribe.

Alright.

[...]
----- ---------------- Fri Oct 25 02:14:07 CEST 2013 Connected.
>>>> ---------------- ---------------- Fri Oct 25 02:14:07 CEST 2013
>>>> Sent: ---------------- Message=Subscribe To=Identities End
>> 
>> I guess it makes sense that it needs no more information if there
>> are three predetermined things a client can subscribe to.
> 
> ?

That is my reaction from being very surprised at the lack of filters. I
don't know that it adds any more to discuss.

[...]
>> Why are both Identity and ID specified as the same thing? I was
>> under the impression is is an identity ID - would just ID be okay?
>>  IIRC it's already not going to be backwards-compatible with
>> existing clients due to the full stops between key segments.
> 
> Yes it is for backwards compatibility even though it still doesn't
> match what existing clients expect: The low-level
> backwards-comptaible code which produces the stuff after the dot does
> not know about the high-level stuff which adds the dot. Basically,
> there is a function "addIdentityFields(..., String prefix, String 
> suffix)". Of course I could have chosen to have the function contain
> some weird logic which only adds certain fields for certain
> prefix/suffix combinations but I think thats ugly because it doesn't
> cleanly separate code into different areas of concern. Arguably you
> already did this with part of the function, but I really didn't want
> it to get even more complicated.

If it doesn't match what existing clients expect why include it?

>> What are Contexts.Amount and Properties.Amount for? The number of
>> each that are given? I think "Count" might be a clearer name.
> 
> I have been using "amount" as a synonym for "count" for years :( Is
> it really not a synonym? I'm not a English native speaker, sorry.

No worries. English makes no sense.

>>>> ---------------- Fri Oct 25 02:15:20 CEST 2013 Received:
>>>> ---------------- Message=IdentityChangedNotification 
>>>> AfterChange.Context0=Introduction
>> 
>> I think a key of AfterChange.Context.0 would be preferable for 
>> consistent separation of the number. Why isn't there an 
>> AfterChange.Context (without the trailing 0) like the other
>> fields?
> 
> Legacy as well. orry. I should have stripped all  legacy stuff from
> the original message. The most recent code path produces the 
> following (which you can see below is also part of the original
> message): "AfterChange.Identities.0.Contexts.0.Name=Introduction"
> 
>>>> AfterChange.CurrentEditionFetchState=Fetched 
>>>> AfterChange.CurrentEditionFetchState0=Fetched
>> 
>> I'm confused why these and others are duplicated save for a 0 on
>> the end of the key.
> 
> Both those fields are also legacy. We have 3 syntaxes of which 2 are
> legacy. The one without 0 is from the oldest code path. Then someone
> suggested to number everything so parsers can eat both versions
> (you?). Then it was suggested to split everything with a dot.

As far as I can tell these legacy fields couldn't be read by an existing
client anyway. Is the thought that they ease transition for existing
clients?

>> Where are the meanings of the possible values for fields like this
>> documented?
> 
> I was reluctant to bloat the "Subscribe" documentation with full
> documentation of the Identity/Trust/Score syntax. So there currently
> is no documentation :| You could enforce me to write one at the
> underlying functions which generate the SFS data by asking me to do
> it.
> 
> But it can be clearly seen what is the most recent syntax by looking
> at the reference parser implementations IdentityParser / TrustParser
> / ScoreParser which are member classes of
> FCPClientReferenceImplementation: 
> https://github.com/freenet/plugin-WoT- 
> staging/blob/master/src/plugins/WebOfTrust/ui/fcp/FCPClientReferenceImplementation.java

Cool.

[...]
>>>> AfterChange.Type=OwnIdentity AfterChange.Type0=OwnIdentity
>> 
>> Would this API change be a place to change the name to
>> LocalIdentity?
> 
> $ grep -R OwnIdentity WebOfTrust/src | wc -l 508
> 
> I don't think that we can/should ever change it. Are you 100% sure
> that it doesn't make any sense in terms of the English language?

It is far ferom grammatically correct, but it does give a sense of
something like "your own identity." I am not suggesting that the
internal class name change. I am suggesting that it is called
LocalIdentity in any external interface, at the very least in the web
interface.

>>>> AfterChange.Contexts.Amount=1 
>>>> AfterChange.Contexts.0.Name=Introduction 
>>>> AfterChange.Contexts0.Amount=1 
>>>> AfterChange.Contexts0.Context0=Introduction
>> 
>> I notice - for example - "Contexts" is plural and in the initial
>> sync message "Identity" is singular.
> 
> I cannot find the singular you are talking about.

"Identities.2.Identity"
              ^

>> It'd make sense to me to make the components consistently one way
>> or another. I have a preference for singular because it's shorter.
> 
> It is plural to: - because the code contains something like 
> "subscribeTo(SubscriptionType.Identities)" and I did want the enum
> names to match the string literals and "subscribeTo(Identity)" makes
> less sense - it DOES contain multiple identities.
> 
[...]
>>>> AfterChange.Identities.0.Type=OwnIdentity 
>>>> AfterChange.Identities.0.Contexts.Amount=1 
>>>> AfterChange.Identities.0.Contexts.0.Name=Introduction
>> 
>> The ".Name" part seems unnecessary to me.
> 
> Mmh I think that is too little of a change for doing actual touching
> of the code. And contexts do might receive additional attributes once
> more complex stuff is implemented such as per-context trust. 
> [Per-context trust might allow you to rate the behavior of identities
> only for certain client applications. While it would be an
> optimization in some areas, it would probably be a full rewrite of
> the most of WOT, and is unlikely to happen. It has been demanded by
> Matthew rather often though for the performance impact. I think it
> would overcomplicate things. Further, either someone is a spammer or
> he isn't. If a person spams in one client application, why would you
> want to trust him in another?]
> 
>>>> AfterChange.Identities.0.Properties.Amount=1 
>>>> AfterChange.Identities.0.Properties.0.Name=IntroductionPuzzleCount
>>>> AfterChange.Identities.0.Properties.0.Value=10
>> 
>> How about
>> AfterChange.Identity.0.Property.IntroductionPuzzleCount=10 ?
> 
> Mmh nice catch. Properties are key/value pairs, SFS as well, so you
> suggest to use the key as the key and the value as the value - good
> idea.
> 
> But it is susceptible to mismatch of allowed string contents in the
> key-space of properties and the key-space of SFS. Typically,
> key/value implementations chose well-definied, small allowed spaces 
> for keys, while allowing arbitrary crap in the values. So the as-is
> code guards againts SFS being more restrictive in key-space than
> properties by only using a well-defined key of "Name" / "Value",
> while shoving all user-definied data into the value only. Remember,
> properties are downloaded from the network, their keys are at
> arbitrary choice of the users. There is some limiting, but I still
> would have to review it.

Is the worry that it would give more ways for WoT properties files to be
invalid or annoying to parse, such as having = in the key?

>>>> AfterChange.Identities.0.Property0.Name=IntroductionPuzzleCount
>>>> AfterChange.Identities.0.Property0.Value=10
>>>> AfterChange.Properties.Amount=1 
>>>> AfterChange.Properties.0.Name=IntroductionPuzzleCount 
>>>> AfterChange.Properties.0.Value=10 
>>>> AfterChange.Properties0.Amount=1 
>>>> AfterChange.Properties0.Property0.Name=IntroductionPuzzleCount 
>>>> AfterChange.Properties0.Property0.Value=10 
>>>> AfterChange.Property0.Name=IntroductionPuzzleCount 
>>>> AfterChange.Property0.Value=10 BeforeChange.Type=Inexistent 
>>>> BeforeChange.Type0=Inexistent 
>>>> BeforeChange.Identities.0.Type=Inexistent
>> 
> 
>> Why isn't there a BeforeChange for every AfterChange?
> 
> Because it "inexistent" that the whole object did not exist, not that
> a member value did not exist. It means that before there was no such
> identity / trust / score before (or after). So I chose one arbitrary
> field to indicate that the whole object didn't exist. I used the one
> which sounded the most synonymous with the whole object while also
> being a field which can never be null for an existing object.
> Identity objects always have a "Type", trusts and scores always have
> a "Value", so I chose those for indicating inexistance.
> 
> If you can come up with a field name which isn't used for any actual
> data and is good for indicating whether it exists or not, suggest it
> please. Before you think about it, read my reply to that please:

>> It looks like "inexistent" is actually a word, but it's not one
>> that I knew. I'd have expected "nonexistent." T his may be a place
>> where including the "before" value can lead to odd corner cases,
>> and I wonder if it's worth including. Instead of a word, would it
>> make sense to have an empty string?
> 
> Maybe I should just set "BeforeChange.Identities.Amount=0" to make
> inexstance very clear, eliminating the whole need for any attribute
> fields BeforeChange.Identities.0.* of the non-existant identity?

If I'm understanding it correctly that strikes me as a special-case that
might complicate things.

> Thanks for your long review and for reading my very long reply :)

:)

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Devl mailing list
Devl@freenetproject.org
https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl

Reply via email to