[
https://issues.apache.org/jira/browse/CASSANDRA-3668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13262708#comment-13262708
]
Sylvain Lebresne commented on CASSANDRA-3668:
---------------------------------------------
* I don't think the 'retries' field should be moved from StreamInSession to
IncomingStreamReader. The ISR is attached to the streaming of one file, so when
the node on the other side does a retry, a new ISR will be created (while the
StreamInSession persists until closed), and so retries field will be reseted
and we will retry infinitely. But it probably does mean it'll make more sense
to have one retry counter per-file (changing StreamInSession.currentFiles to a
Map<String, Integer> maybe?)
* Currently, every messages related to the streaming goes through the ISR
socket *except* for the message of session failure (in
StreamInSession.closeInternal). Could be nice to use this ticket to fix that
(but if you prefer moving to another ticket that's ok).
* StreamOutSession.begin() shouldn't assume the queue has at least one element
(at least the current code don't expect it)
Smallish remarks:
* MessagingService: Looking at ThreadPoolExecutor code it seems that this is
harmless but bumping the executor core pool size would make it exceed the max
pool size. I'd be in favor of changing the executor definition:
{noformat}
executor = new DebuggableThreadPoolExecutor(0,
1,
1,
TimeUnit.SECONDS,
new
LinkedBlockingQueue<Runnable>(),
new NamedThreadFactory("Streaming
to " + to));
{noformat}
to
{noformat}
executor = new DebuggableThreadPoolExecutor(1,
Integer.MAX_VALUE,
1,
TimeUnit.SECONDS,
new
LinkedBlockingQueue<Runnable>(),
new NamedThreadFactory("Streaming
to " + to));
{noformat}
as it's cleaner imho (it doesn't use the non-documented fact that a
corePoolSize of 0 still starts a core thread for instance) and equivalent
(since DTPE timeouts core threads).
* IncomingTcpConnection: sets the name when starting streaming but don't unset
it. Probably not worth changing the name, the thread name is probably not the
best place anyway to log activity (but it's a good idea to set a meaningful
name in the first place).
* In FileStreamTask.runMayThrow, instead of logging an error, we could wrap the
IOException in a new one with the message. That way we wouldn't have 2 messages
in the log for the same error.
* Would feel more coherent to me to move the sending of the SESSION_FINISHED
message in ISR (for instance SIS.closeIfFinished could return a boolean if the
session is indeed finished (and we would just have to pass the file name to
this method instead of the ISR instance)).
* Not this patch fault but in SIS.getIncomingFiles(), adding the currentFiles
is useless
* In StreamOutSession.begin() there is a commented debug line (could be removed)
* I think the patch needs one more rebase (after CASSANDRA-4174)
> Performance of sstableloader is affected in 1.0.x
> -------------------------------------------------
>
> Key: CASSANDRA-3668
> URL: https://issues.apache.org/jira/browse/CASSANDRA-3668
> Project: Cassandra
> Issue Type: Bug
> Components: API
> Affects Versions: 1.0.0
> Reporter: Manish Zope
> Assignee: Yuki Morishita
> Priority: Minor
> Labels: streaming
> Fix For: 1.1.1
>
> Attachments: 0001-Allow-multiple-connection-in-StreamInSession.patch,
> 0002-Allow-concurrent-stream-in-StreamOutSession.patch,
> 0003-Add-threads-option-to-sstableloader.patch,
> 3688-reply_before_closing_writer.txt, sstable-loader performance.txt
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> One of my colleague had reported the bug regarding the degraded performance
> of the sstable generator and sstable loader.
> ISSUE :- https://issues.apache.org/jira/browse/CASSANDRA-3589
> As stated in above issue generator performance is rectified but performance
> of the sstableloader is still an issue.
> 3589 is marked as duplicate of 3618.Both issues shows resolved status.But the
> problem with sstableloader still exists.
> So opening other issue so that sstbleloader problem should not go unnoticed.
> FYI : We have tested the generator part with the patch given in 3589.Its
> Working fine.
> Please let us know if you guys require further inputs from our side.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
