Skip to content

Provide Solr-7.x-plugin#2853

Closed
dmsolr wants to merge 5 commits intoapache:masterfrom
dmsolr:solr-7.x-plugin
Closed

Provide Solr-7.x-plugin#2853
dmsolr wants to merge 5 commits intoapache:masterfrom
dmsolr:solr-7.x-plugin

Conversation

@dmsolr
Copy link
Copy Markdown
Member

@dmsolr dmsolr commented Jun 11, 2019

Please answer these questions before submitting pull request

Copy link
Copy Markdown
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest move this to optional plugin and add a document for this. Because this plugin should only work when agent installed in ElasticSearch server side, right?

@dmsolr
Copy link
Copy Markdown
Member Author

dmsolr commented Jun 11, 2019

The situation on the server side is more complicated than SolrJ.
I need some advice how to name operationName and other.

It will put the trace id into Solr MDC. Then Solr logs it in SLOW logger or where you want to by Log4j configuration file.

image
image

@dmsolr
Copy link
Copy Markdown
Member Author

dmsolr commented Jun 11, 2019

@wu-sheng yes, it works in Solr Server, likes ElasticSearch server-side.

* [transport-client](https://github.com/elastic/elasticsearch/tree/master/client/transport) 5.2.x-5.6.x
* [SolrJ](https://lucene.apache.org/solr) 7.0.0-7.7.1
* [Solr](https://github.com/apache/lucene-solr) 7.0.0-7.7.1
* [SolrJ](https://github.com/apache/lucene-solr/tree/master/solr/solrj) 7.0.0-7.7.1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add Solr Server and versions as a subtype here.

* [Elasticsearch](https://github.com/elastic/elasticsearch)
* [transport-client](https://github.com/elastic/elasticsearch/tree/master/client/transport) 5.2.x-5.6.x
* [SolrJ](https://lucene.apache.org/solr) 7.0.0-7.7.1
* [Solr](https://github.com/apache/lucene-solr) 7.0.0-7.7.1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the version.

@wu-sheng wu-sheng added agent Language agent related. feature New feature plugin Plugin for agent or collector. Be used to extend the capabilities of default implementor. labels Jun 11, 2019
@wu-sheng wu-sheng requested a review from IanCao June 11, 2019 09:40
@wu-sheng wu-sheng added the TBD To be decided later, need more discussion or input. label Jun 11, 2019
@wu-sheng
Copy link
Copy Markdown
Member

Could you explain more about how many spans created for each request? And what are they?

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.004%) to 17.242% when pulling da99c25 on dmsolr:solr-7.x-plugin into ec731b2 on apache:master.

@dmsolr
Copy link
Copy Markdown
Member Author

dmsolr commented Jun 11, 2019

Here is a screenshot of solr collection. Obviously, it consists of 2 shards with 2 replicas per shard.

image

In query, we omit the replicas. One shard become a coordinator, when it receives a query from user. Because it is has a part of data in collection, It launch a request and send to others shards. Then coordinator have to merge the result that comes from all shards. In real scenario, it has serveral stages in usually.

image
It seems like above picture.

In update, the documents need to be routed and send to corresponding shard. Then shard will copy it to they replicas.

@dmsolr
Copy link
Copy Markdown
Member Author

dmsolr commented Jun 11, 2019

Solr is a distributed database.

@dmsolr
Copy link
Copy Markdown
Member Author

dmsolr commented Jun 11, 2019

@kezhenxu94, Do you have anything else for the explaination?

@wu-sheng
Copy link
Copy Markdown
Member

OK. I think it is clear enough for the Solr server. Then let's talk about how this goes in topology and trace.

  1. Does topology show Solr server cluster as a single one nodes? Like other distributed system.
  2. Do you plan to show RPC between Solr server nodes?

I think the decision is based on the answers of the above questions.

@dmsolr
Copy link
Copy Markdown
Member Author

dmsolr commented Jun 11, 2019

@wu-sheng , Sorry, I have a mistake that I configured the agent.service_name in smae name.
In fact, they work in cluster.
By the way, Solr still uses http. So I define the SpanLayer as http.

image

image

image

@wu-sheng
Copy link
Copy Markdown
Member

@TinyAllen I noticed in this case, there are instances of one service calling each others, the topology shows a little strange, please take a look.

@wu-sheng
Copy link
Copy Markdown
Member

@dmsolr If the client SolrJ is instrumented, do the topology and metrics still make sense? I notice you are identifying the different shard as different service? Then which peer is SolrJ using?

Such as Solr server nodes are 10.5.1.23:1000(node1), 10.5.1.53:1000(node2). What is SolrJ plugin peer? I am assuming something like 10.5.1.23:1000,10.5.1.53:1000? If so, the topology can't show perfectly, because backend can't tell 10.5.1.23:1000,10.5.1.53:1000 is alias name of node1 or node2.

@kezhenxu94
Copy link
Copy Markdown
Member

kezhenxu94 commented Jun 12, 2019

Then which peer is SolrJ using?

Such as Solr server nodes are 10.5.1.23:1000(node1), 10.5.1.53:1000(node2). What is SolrJ plugin peer?

@wu-sheng The peer of SolrJ Plugin may not be always the same since there're something like 'load balancing' in the request. The SolrJ would contact the zookeeper first to get a Solr node and then send the request to that node (afterwards the request may require communication between Solr nodes such as aggregation), the request may not always be routed to the same node. Am I right? @dmsolr

@dmsolr
Copy link
Copy Markdown
Member Author

dmsolr commented Jun 12, 2019

Great, @kezhenxu94 .
Communication between nodes use SolrJ withou SolrJ-plugin.
You mean there is a problem when SolrJ-plugin works with Solr-plugin in one node, right. @wu-sheng

@Override
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
Object ret) throws Throwable {
if (method.getName().equals("finishStage")) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm a bit confused about if (method.getName().equals("distributedProcess")) in before method and if (method.getName().equals("finishStage")) { in after method

this two method name should be same.but this logic judgment is not

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This span start at distributedProcess() and stop at finishStage().
But I'm pretty sure they always come in pairs.


@Override
public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes, Throwable t) {
if (ContextManager.isActive()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!request.getPath().startsWith(SKIP_ADMIN_PREF) should be here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I notice that. Maybe it is not required. In usually, there is no active span which matches this condition here. (Unless other interceptors create.)

That there is a active span in context, not matter of who created, logs the exception when it is in the method with catching exception. I think.

@wu-sheng
Copy link
Copy Markdown
Member

You mean there is a problem when SolrJ-plugin works with Solr-plugin in one node, right.

Could you test about SolrJ plugin works with one node Solr server and multiple nodes cluster? Especially for Topology. I highly doubt the topology will make sense if you do so when APP with Solr cluster.

@dmsolr
Copy link
Copy Markdown
Member Author

dmsolr commented Jun 12, 2019

I have tried it seems like ok.

image

@wu-sheng
Copy link
Copy Markdown
Member

What is the user accessing all nodes? Maybe because you are using browser access Solr server?

@dmsolr
Copy link
Copy Markdown
Member Author

dmsolr commented Jun 13, 2019

Yes, I accessed all the nodes through the browser.
Accessing Solr via browser only connects to one node.
But SolrJ can access all nodes in a request, expecially writting data. This is because the node to which the document should be written is usually known. If partition rule is given.

@wu-sheng
Copy link
Copy Markdown
Member

OK, if no peer looks like ip1:port1,ip2:port2... in SolrJ plugin, I think that is fine.

@wu-sheng
Copy link
Copy Markdown
Member

cc #2871

@wu-sheng
Copy link
Copy Markdown
Member

SolrJ plugin has been reverted, have to, because it breaks the agent.

@kezhenxu94 kezhenxu94 mentioned this pull request Jun 17, 2019
3 tasks
@asfgit
Copy link
Copy Markdown

asfgit commented Jun 23, 2019

FAILURE

@asfgit
Copy link
Copy Markdown

asfgit commented Jun 23, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/skywalking-e2e/11/

@dmsolr
Copy link
Copy Markdown
Member Author

dmsolr commented Jul 4, 2019

Cases List

Case description Status Cases
solr 12 passed. 0 failed click me

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Jul 4, 2019

You need to fix conflict first.

@dmsolr
Copy link
Copy Markdown
Member Author

dmsolr commented Jul 4, 2019

The PR has delayed for long time, so I works in a new branch. This branch lose some commits. Could I close this and open another PR?

@dmsolr dmsolr closed this Jul 4, 2019
@dmsolr
Copy link
Copy Markdown
Member Author

dmsolr commented Jul 4, 2019

move to #3002

@dmsolr dmsolr deleted the solr-7.x-plugin branch July 10, 2019 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Language agent related. feature New feature plugin Plugin for agent or collector. Be used to extend the capabilities of default implementor. TBD To be decided later, need more discussion or input.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants